Uploads: failed and abandoned resumable uploads count against the 16-active limit for 24 h #96

New issue

Closed

opened 2026-09-25 15:32:15 +00:00 by kayg · 1 comment

kayg commented

2026-09-25 15:32:15 +00:00

Owner

Fix: a failed finalization (412 or any terminal error) releases the slot immediately and deletes the staging bytes; abandoned uploads expire on an idle timeout (e.g. no PATCH for 15 min) rather than the 24 h lifetime; the limit counts only uploads that received bytes recently. Add tests: 20 failed finalizations in a row, then a normal upload succeeds; an idle upload frees its slot after the idle timeout (use an injectable clock).

Found by the sync-load job (#88). When a resumable upload fails with 412 at finalization, or a browser tab abandons one, the server keeps it as active for its full 24 h lifetime; each user may have at most 16 active uploads, so after enough failures every upload answers '429 too many active uploads' for up to a day. This locks a user out of uploads (availability), and the adversarial round-2 harness trips it after about 7 runs against one server. Fix: a failed finalization (412 or any terminal error) releases the slot immediately and deletes the staging bytes; abandoned uploads expire on an idle timeout (e.g. no PATCH for 15 min) rather than the 24 h lifetime; the limit counts only uploads that received bytes recently. Add tests: 20 failed finalizations in a row, then a normal upload succeeds; an idle upload frees its slot after the idle timeout (use an injectable clock).

kayg commented

2026-09-25 16:41:37 +00:00

Author

Owner

Fixed on branch job/upload-slots (based on job/sync-load). The branch is not merged or pushed.

Root cause

expires_at was a fixed lifetime of 24 h. A failed finalization (412, 403 or any other error after the last byte) left the row in place. An abandoned upload also stayed. Both counted against the limit of 16 until they expired.
The cleanup job could never remove orphan staging, because Root::upload_ids used the public listing, which hides .system/.
The cleanup job uses a second FilesState, so it did not share the per-upload locks of the router.

Fix

finalize() wraps finish(). On any error it deletes the upload row and its staging at once, and then returns the error.
expires_at is now a sliding idle lease: 15 min after creation or after the last accepted chunk, and never more than 24 h after creation. HEAD does not renew the lease. The limit of 16 and the quota reservation count only live leases.
Migration 11 gives existing uploads one fresh idle window.
The cleanup job runs every 15 min (it ran every hour before). The upload lock registry is shared by the whole process.
upload_ids uses the internal listing.
FilesState::with_clock takes a calternal_db::Clock for tests.
Why 15 min: the idle timeout must be longer than the time to send one chunk. A web chunk is 4 MiB, so the web client needs at least 4.6 KB/s. A sync chunk is 1 MiB, so the sync client needs at least 1.2 KB/s.

Clients

The server answers 404 for an expired upload.
Web: 404 or 410 on HEAD or on PATCH starts a new upload from byte 0, at most 3 times in one run. A 412 now shows its own message.
Sync: 404 or 410 on upload HEAD, PATCH or DELETE maps to Error::UploadExpired. Before, a 410 was read as CursorExpired. A PATCH that gets this answer starts a new upload with the same snapshot, at most 3 times.

Tests

Files: failed_finalizations_release_their_slots_at_once (20 failures in a row, then a normal upload), idle_upload_frees_its_slot_after_the_idle_timeout (fake clock), slow_upload_keeps_its_slot_while_bytes_arrive (also checks the 24 h cap).
fs: upload_ids_lists_staging_in_the_reserved_tree.
Sync: an_expired_upload_restarts_from_byte_zero, restarts_stop_after_the_limit, an_unknown_upload_reads_as_expired_not_as_a_cursor_or_server_error.
Web: uploads.svelte.test.ts (4 cases).
Adversarial round 2: new section uploadslots. It runs 48 failed finalizations (serial and parallel) and 20 stale If-Match replacements. Then it checks that each released upload answers 404, that 16 new uploads get slots, that the 17th gets 429, that a normal upload works, and that no staging is left.

Adversarial runs
The host was at load 30 on 8 cores during the runs, because of other jobs. The only findings were SLOW latency checks between 5.4 and 7.1 s, on different probes in each run. There were no 5xx responses, crashes or wrong statuses. The uploadslots checks passed in all three runs.

Fixed on branch `job/upload-slots` (based on `job/sync-load`). The branch is not merged or pushed. **Root cause** - `expires_at` was a fixed lifetime of 24 h. A failed finalization (412, 403 or any other error after the last byte) left the row in place. An abandoned upload also stayed. Both counted against the limit of 16 until they expired. - The cleanup job could never remove orphan staging, because `Root::upload_ids` used the public listing, which hides `.system/`. - The cleanup job uses a second `FilesState`, so it did not share the per-upload locks of the router. **Fix** - `finalize()` wraps `finish()`. On any error it deletes the upload row and its staging at once, and then returns the error. - `expires_at` is now a sliding idle lease: 15 min after creation or after the last accepted chunk, and never more than 24 h after creation. HEAD does not renew the lease. The limit of 16 and the quota reservation count only live leases. - Migration 11 gives existing uploads one fresh idle window. - The cleanup job runs every 15 min (it ran every hour before). The upload lock registry is shared by the whole process. - `upload_ids` uses the internal listing. - `FilesState::with_clock` takes a `calternal_db::Clock` for tests. - Why 15 min: the idle timeout must be longer than the time to send one chunk. A web chunk is 4 MiB, so the web client needs at least 4.6 KB/s. A sync chunk is 1 MiB, so the sync client needs at least 1.2 KB/s. **Clients** - The server answers 404 for an expired upload. - Web: 404 or 410 on HEAD or on PATCH starts a new upload from byte 0, at most 3 times in one run. A 412 now shows its own message. - Sync: 404 or 410 on upload HEAD, PATCH or DELETE maps to `Error::UploadExpired`. Before, a 410 was read as `CursorExpired`. A PATCH that gets this answer starts a new upload with the same snapshot, at most 3 times. **Tests** - Files: `failed_finalizations_release_their_slots_at_once` (20 failures in a row, then a normal upload), `idle_upload_frees_its_slot_after_the_idle_timeout` (fake clock), `slow_upload_keeps_its_slot_while_bytes_arrive` (also checks the 24 h cap). - fs: `upload_ids_lists_staging_in_the_reserved_tree`. - Sync: `an_expired_upload_restarts_from_byte_zero`, `restarts_stop_after_the_limit`, `an_unknown_upload_reads_as_expired_not_as_a_cursor_or_server_error`. - Web: `uploads.svelte.test.ts` (4 cases). - Adversarial round 2: new section `uploadslots`. It runs 48 failed finalizations (serial and parallel) and 20 stale If-Match replacements. Then it checks that each released upload answers 404, that 16 new uploads get slots, that the 17th gets 429, that a normal upload works, and that no staging is left. **Adversarial runs** The host was at load 30 on 8 cores during the runs, because of other jobs. The only findings were SLOW latency checks between 5.4 and 7.1 s, on different probes in each run. There were no 5xx responses, crashes or wrong statuses. The `uploadslots` checks passed in all three runs.

kayg referenced this issue from a commit

2026-09-25 18:47:22 +00:00

fix(files): free upload slots on failed finalization and idle timeout

kayg referenced this issue from a commit

2026-09-25 18:47:22 +00:00

fix(sync): restart an expired tus upload instead of failing the file

kayg referenced this issue from a commit

2026-09-25 18:47:22 +00:00

fix(web): restart an upload the server released instead of failing it

kayg referenced this issue from a commit

2026-09-25 18:47:22 +00:00

test(adversarial): burn upload slots with failing finalizations