Skip to content

Fix block upload worker leaks after an error - #8

Open
ClaudiuSchuster wants to merge 2 commits into
rclone:masterfrom
oss-singularity:fix/drain-upload-block-workers
Open

Fix block upload worker leaks after an error#8
ClaudiuSchuster wants to merge 2 commits into
rclone:masterfrom
oss-singularity:fix/drain-upload-block-workers

Conversation

@ClaudiuSchuster

@ClaudiuSchuster ClaudiuSchuster commented Aug 27, 2026

Copy link
Copy Markdown

Problem

uploadPendingBlocks returned as soon as it received the first block-upload error. The remaining goroutines could then block while sending to the unbuffered result channel, still holding their weighted-semaphore slots. Repeated retries progressively consumed all block-upload slots and left later uploads unable to start.

The acquisition-error path also continued to a deferred Release even though no slot had been acquired.

Fix

  • Buffer one result per block.
  • Receive every worker result before returning the first error.
  • Return immediately when acquiring a slot fails.
  • Add a regression test that repeats failing batches and then acquires the full semaphore capacity.

Verification

Validated with Go 1.26 and golangci-lint 2.9.0, matching the repository workflow:

golangci-lint run --timeout=180s
go test -v ./...
go test -v -race ./...
go test -race -run '^TestCollectUploadErrorsReleasesAllWorkersAfterFailure$' -count=100 ./...
go build ./...

All commands pass. The repository's credential-gated Proton integration tests are skipped by their existing guard; all unit tests, including the new regression test, run successfully.

The fork's repository workflow also passes on this exact commit: Lint and Test.

Receive every block upload result before returning the first error so all workers can release their semaphore slots. Buffer the result channel and return immediately when slot acquisition fails.

Add a regression test which repeats failing batches and then acquires the full semaphore capacity.
Retry only failed encrypted blocks with fresh upload links and bounded context-aware backoff. Preserve successful blocks and return terminal or exhausted errors without replaying the complete file stream.

Refs oss-singularity/proton-drive-linux#42
@ClaudiuSchuster

Copy link
Copy Markdown
Author

Additional real-world validation from the downstream PDrive integration:

  • the exact worker-drain change was deployed in a checksum-pinned rclone build during one 30.4 GiB VFS upload;
  • six separated, same-process Proton block-upload 502 cycles occurred after that build became active;
  • the same rclone process continued without a systemd restart and payload progress resumed after the failures;
  • the upload has now completed with an empty queue and VFS cache.

This supports the semaphore-release behavior under repeated real backend failures, beyond the unit and race tests already listed in the PR.

A separate bounded retry for only the failed encrypted blocks is tracked in oss-singularity/proton-drive-linux#42 and was validated in oss-singularity/Proton-API-Bridge#2. I am deliberately keeping that follow-up out of this PR so this worker-lifecycle fix remains focused. Once this prerequisite lands, the retry can be proposed upstream as a single independent commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant