feat: improve upload management: FIFO slicing + session-aware adaptiv…#195
Conversation
paul-fresquet
left a comment
There was a problem hiding this comment.
Good job 👍 Here are a few comments
src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadProcessor.cs
Show resolved
Hide resolved
|
|
||
| public interface IUploadSlicingManager | ||
| { | ||
| Task Enqueue(Func<Task> startSlicingAsync); |
There was a problem hiding this comment.
note: the signature might be: Task<UploadProgressState> Enqueue(SharedFileDefinition sharedFileDefinition).
UploadSlicingManager would then instanciate the FileSlicer, the UploadProgressState and return the UploadProgressState.
src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs
Show resolved
Hide resolved
src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadSlicingManager.cs
Outdated
Show resolved
Hide resolved
src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadSlicingManager.cs
Show resolved
Hide resolved
src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileSlicer.cs
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a centralized upload slicing queue to enforce FIFO ordering and bounded concurrency across uploads, while also implementing session-aware adaptive parameter resets to improve upload management predictability and consistency.
- Adds
UploadSlicingManagerto centralize slice processing with FIFO ordering and configurable concurrency limits - Implements session-aware adaptive upload controller that resets parameters when sessions change
- Refactors upload architecture to use the centralized slicing manager instead of direct slice processing
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| UploadSlicingManager.cs | New centralized manager for FIFO slice processing with bounded concurrency |
| AdaptiveUploadController.cs | Enhanced with session-aware reset capability and improved downscaling logic |
| FileUploadProcessor.cs | Refactored to use slicing manager instead of direct FileSlicer dependency |
| FileSlicer.cs | Added backpressure control to prevent excessive pending slices |
| FileUploadCoordinator.cs | Changed from bounded to unbounded channel for slice queueing |
| Multiple test files | Updated tests to accommodate new architecture and dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadSlicingManager.cs
Show resolved
Hide resolved
src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadSlicingManager.cs
Show resolved
Hide resolved
src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileSlicer.cs
Show resolved
Hide resolved
c87dd0c
into
feature/improve-upload-management



Summary
Introduces a centralized slicing queue to enforce FIFO ordering and bounded concurrency across uploads, and resets adaptive upload parameters on session changes.
Key changes
UploadSlicingManager: AddsIUploadSlicingManagerandUploadSlicingManagerusing aChannel<Func<Task>>, FIFO processing, and max concurrency of 2; ignores stale work after session resets.AdaptiveUploadControllersubscribes toISessionService.SessionObservableand fully resets chunk size, parallelism, and window.AdaptiveUploadControllerandUploadSlicingManagerinSingletonsModule; resolves and injectsIUploadSlicingManagerinFileUploadProcessorFactory.FileUploadProcessorenqueues slicing viaIUploadSlicingManagerinstead of starting it directly to ensure ordered execution.SliceAndEncryptAdaptiveAsync, injects and stubsIUploadSlicingManager, and wiresISessionServicemock for the controller.Rationale
Notes
MAX_CONCURRENT_SLICESis set to 2 and can be tuned later.