-
Notifications
You must be signed in to change notification settings - Fork 0
Add Streaming mutex #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a streaming upload mechanism with mutex-based concurrency control for file uploads. The main purpose is to introduce a third upload strategy that processes files in a streaming pipeline fashion, where each file can move through different phases (MD5 generation, URL generation, upload) independently rather than waiting for all files to complete each phase before moving to the next.
Key changes:
- Adds streaming upload strategy with real-time progress tracking using mutex for thread-safe state management
- Introduces comprehensive UI updates to compare all three upload strategies with performance metrics
- Adds detailed blog documentation explaining concurrency control concepts in both Portuguese and English
Reviewed Changes
Copilot reviewed 8 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/useCases/useStreamingUpload.ts | New streaming upload hook with mutex-protected counters and progress tracking |
src/useCases/useAsyncBatchUpload.ts | New batch upload hook for async parallel processing |
src/types/uploadProgress.ts | Type definitions for file progress tracking and upload hook interfaces |
src/App.tsx | Major UI overhaul to support strategy selection and performance comparison tables |
package.json | Adds async-mutex dependency for concurrency control |
blog/PT-BR.md | Comprehensive Portuguese blog post explaining concurrency concepts |
blog/EN-US.md | English translation of the blog post |
README.md | Updated documentation with usage instructions and troubleshooting |
if (result.ok) { | ||
completedCountRef.current += 1; | ||
console.log( | ||
`MUTEX ACCESS: Incremented completed count to ${completedCountRef.current}` |
Copilot
AI
Jul 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug console.log statements should be removed from production code. Consider using a proper logging library or removing these logs before deployment.
Copilot uses AI. Check for mistakes.
} else { | ||
failedCountRef.current += 1; | ||
console.log( | ||
`MUTEX ACCESS: Incremented failed count to ${failedCountRef.current}` |
Copilot
AI
Jul 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug console.log statements should be removed from production code. Consider using a proper logging library or removing these logs before deployment.
Copilot uses AI. Check for mistakes.
? batch | ||
: streaming; | ||
|
||
// TODO remove all UI logic and remove unnecessary hooks |
Copilot
AI
Jul 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO comment indicates incomplete refactoring. The UI logic should be extracted into separate components or hooks to improve maintainability and separation of concerns.
Copilot uses AI. Check for mistakes.
const failedResponse = new Response(null, { | ||
status: 500, | ||
statusText: error instanceof Error ? error.message : "Unknown error", | ||
}); | ||
|
||
await setResult(index, failedResponse); | ||
return failedResponse; |
Copilot
AI
Jul 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a mock Response object for error handling is not ideal. Consider using a custom error type or a proper error handling mechanism instead of fabricating HTTP responses.
const failedResponse = new Response(null, { | |
status: 500, | |
statusText: error instanceof Error ? error.message : "Unknown error", | |
}); | |
await setResult(index, failedResponse); | |
return failedResponse; | |
const failedError = { | |
status: 500, | |
message: error instanceof Error ? error.message : "Unknown error", | |
fileName: file.name, | |
}; | |
await setResult(index, failedError); | |
return failedError; |
Copilot uses AI. Check for mistakes.
66d39d9
to
80d47f0
Compare
No description provided.