perf(upload): spread a batch over upload_workers CLI processes - #10
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #9 (shares the mutation retry path). Retarget to main once #9 merges.
Why
Upload is 66% of every run: one
proton-drive filesystem uploadper batch, measured at 0.208 s per file plus 12.8 MB/s, on a link that does two orders of magnitude more. The CLI has no concurrency flag, so the only lever is several processes.Change
proton.upload_workers(default 1, so nothing changes until it is raised) and the run flagUPLOAD_WORKERS=nafter the double dash or in the workflow's vars input, for one run at a time. The experiment is a dispatch withUPLOAD_WORKERS=2, then 4, readingupload_secondsfrombatches.details_json.uploadspreads the batch's top-level staging entries over at most that many groups, heaviest first into the lightest group (bytes plus 2.7 MB per file, the measured per-file cost). Disjoint top-level entries share no folder to create, so-d mergenever races. Each call writes its own report,upload-<batch>-<k>.json, whose first line names the entries it was handed.upload_treeson the provider runs the groups at once, each process from its own copy of the session exactly as the reconcile walk's workers do (WorkerSessions). Worker threads only run the CLI; every attempt is recorded and logged afterwards on the calling thread, which is the one that owns the state. A group the CLI signed out because its token refresh lost the race is re-run once on the copy that won, after that copy is adopted and every copy re-seeded; the session is written back once after adoption. Any group that still fails raises, and the batch fails as today.confirmholds each call's summary against the files and folders under the entries that call was handed: transferred plus skipped plus failed must equal that call's files plus folders, every failure must name a file among them, and the batch confirms only when every call does and every verified file was covered by some call. A failure now marks twins within its own call rather than across the batch, which is strictly narrower. With one worker this is the accounting that ran before, over the whole batch.Test plan
task test: 164 passed. New: two workers split a batch and a failure in one call marks only that call's file; a report that balances but does not cover every file records nothing; two groups run at once and a signed-out group is rescued on the adopted session, with the session written back once; a group that fails for good raisestask lint,task checksyncwithvars: UPLOAD_WORKERS=2, compareupload_secondsper batch against the fitted0.208 * files + bytes / 12.8e6; then 4. If it scales, setupload_workersinconfig/mirror.toml. Watch for "adopted the refreshed Proton session" warnings and for any duplicate folders in the next reconcile