[np5]

#1685 is merged, and this PR is based on main. A large artifact goes up in parts now, and Zentral closes the upload rather than the agent.

Rebased on main after the merge of #1685, which reorganized zentral/utils/storage.py into capabilities, operations and implementations named by backend. The multipart storage code follows that layout now: see the storage section. The turbo commit needed one semantic change: the completion task reads the CRC-64/NVME from the new stat_object() shape, stored["digests"].

storage: the multipart control plane, and the declaration it rests on

Five operations, in the layout of the module: create_multipart_upload(), generate_presigned_part(), list_multipart_parts(), complete_multipart_upload() and abort_multipart_upload(). Each one dispatches to an _s3_* implementation, and refuses another storage with a ValueError. A caller asks a new capability first, file_storage_has_multipart_uploads(), which is true on S3 only. It is a question of its own, and not file_storage_has_presigned_uploads(): GCS can presign a single PUT, and has an S3-compatible XML API for multipart that Zentral does not use.

create_multipart_upload() declares ChecksumAlgorithm: CRC64NVME, and that declaration is the one line of this design that must not be dropped. Without it S3 accepts a wrong whole-object checksum at completion, and HeadObject afterwards reports a perfectly correct one because S3 computed its own — the object looks verified and is not. ChecksumType: FULL_OBJECT and MpuObjectSize genuinely are optional; do not read their optionality as license to drop the algorithm.

CRC-64/NVME rather than SHA-256 because S3 offers a full-object checksum on a multipart upload only for the CRC algorithms: only those compose from part checksums into a checksum of the whole object.

ListParts paginates. Reading a truncated first page as the whole list would assemble a partial object and call it done — and Zentral's own geometry cannot fill a page, which is exactly why a truncated answer must not be mistaken for a complete one.

The presigned part goes through the SigV4 client, like every presign here. A part carries no checksum, so its length is the entire contract, and the legacy V2 presign signs no header at all. Measured again while writing this: a raw client presigns V2 in us-east-1, so this is not hypothetical.

Two ceilings. The calls inside a device request — the metadata read, and the start of a multipart upload — keep the _REQUEST ceiling of seconds and one attempt, which the GCS metadata read also uses; on S3 they go through _s3_request_client(). The calls of a Celery task — listing the parts, and the completion and the abort of the upload — go through _s3_task_client(): ten minutes to read, because S3 documents that the assembly can take several, and three attempts in botocore's standard mode, with exponential backoff and jitter, which ride out an outage of a few seconds. max_attempts in botocore counts retries, not attempts:

max_attempts=0 -> {'mode': 'standard', 'total_max_attempts': 1}
max_attempts=1 -> {'mode': 'standard', 'total_max_attempts': 2}

So both clients set total_max_attempts, the key botocore actually keeps, and the tests assert that key. The S3 client of #1685's metadata read becomes _s3_request_client(), because this PR gives it another caller, starting a multipart upload.

Exceptions of its own. The operations raise StorageUnavailable when the storage did not answer or asked to slow down, MultipartUploadNotFound for an upload id that is gone, MultipartUploadRefused when the parts can never make the declared object, and StorageError for the rest. The S3 implementations translate the errors of botocore, and the GCS metadata read the errors of the GCS library, so stat_object() raises the same exceptions on both storages. Only a connection error or a timeout is StorageUnavailable: botocore's ConnectionError and HTTPClientError, and the transport errors of the GCS library. A missing credential, a missing region or a parameter that the code got wrong is a plain StorageError, which the task logs as an error and not as an outage. Turbo imports nothing from botocore now. The S3 tests replace the client builder instead of priming its cached attribute: priming the attribute worked until the attribute was renamed, and then the calls went to the real endpoint and failed with a 403.

turbo: multipart, and the server closes it

The mint answers mode: "multipart" with the geometry and one signed URL per part. The geometry comes back with the URLs, which is the whole reason it is not published or cached anywhere: the agent sends the same body either way and reads the mode off the answer, so there is nothing to predict, no part size to cache, and no stale-configuration two-step. Multipart is offered only where file_storage_has_multipart_uploads() says so, which is S3. Elsewhere every upload is a single PUT, under the ceiling of the deployment.

The threshold is two part sizes, 128 MB. At one there is no resumability to buy: a part_size + 1 artifact is a two-part upload whose second part is one byte, so losing the first part costs the retransmission that restarting a single PUT would have — the whole multipart bill paid to protect a one-byte tail. At two, the first failure loses at most half the transfer.

A multipart artifact is refused without crc64nvme. sha256 is composite-only on a multipart upload, so the whole-object CRC is the only thing that can validate the assembled object; without it completion would accept whatever arrived. Nothing uses the CRC on a single PUT, and a single PUT is not refused for want of it.

Completion is the server's. The agent posts the three keys that identify the upload and gets a 202; Zentral lists the parts from the storage — never from the wire — and completes with its own credentials. That is the one place the agent would have needed storage-specific knowledge: collecting ETags, building a completion body, and on S3 finding an error inside a 200 OK. It runs in Celery because S3 documents that completion can take minutes, and the agent has no use for the answer anyway. Idempotent, so the call can be retried freely.

The task maps each answer onto the verification axis. A checksum that does not match the bytes is terminal — assembly_failed — because no retry improves it and retrying keeps the parts alive and billed. NoSuchUpload is ambiguous: the storage says it both for an upload that completed already and for one whose parts were aborted, and the object itself is the only thing that tells them apart.

No retries in the task, and a sweep instead. The storage calls of the task ride out an outage of a few seconds. After a longer outage, or another storage error, the upload stays pending, with a warning or an error in the log, and the new sweep_turbo_uploads management command asks again: for an upload whose result arrived at least 45 minutes ago, with no verdict, and at most once every 45 minutes for each upload (--min-age). The window is longer than 30 minutes, the worst case of one completion, so the sweep does not ask again for a completion that is still running, and shorter than the hourly cron, with a margin: with a window equal to the interval, a run that starts a few milliseconds early skips the uploads that the previous run asked for, and the sweep asks every one or two hours by chance. Two tests pin both bounds: a window of 60 fails one, a window of 30 the other. The age is completed_at, when the result arrived, and not updated_at, which any save moves. An upload whose result has not arrived is not swept: the agent can still be sending parts, and a completion then would be an assembly failure, which is final. A completion asked twice does no harm, because the task is idempotent and a verdict is final. An abort that fails is logged and not raised: the lifecycle rule removes the parts.

Safe on several instances. The cron of each worker instance runs the sweep, so two runs can overlap, or follow each other. Each run claims what it asks for in one transaction, like notification_queued_at in the APNS worker: SELECT … FOR UPDATE SKIP LOCKED gives two overlapping runs disjoint rows, and a new nullable JobUpload.last_swept_at (migration 0004) keeps a later run from asking again within the window. No lock to take, and no instance to elect. An advisory lock would not do it: the sweep takes milliseconds, so two crons of the same minute rarely overlap, and both would ask. A TransactionTestCase holds the row lock in a second connection and checks that the sweep skips the row instead of waiting.

Result ingest does not HEAD a multipart row — completion already validated the whole object against the mint. It checks the result against the mint, like the single-PUT path: a result that reports another size or sha256 than its mint describes a second file, and that is a mismatch, even over a verdict that the task already wrote. Then it notices that a completion is still owed (the agent's call can have been lost, and a result is often the first thing to arrive) and drops the parts when the agent reports giving up, or when its result contradicts its mint before any completion: that verdict is final, so nothing would complete the upload, and the parts would otherwise wait for the lifecycle rule.

Three writers, and a decided verdict is final. The ingest writes the status, the mint the geometry and its attempts, and the completion task the verification, each with named fields: the task loads the row minutes before it writes, and a full save from a stale copy would put an axis back. The task writes its verdict with a conditional update, only over pending, so a mismatch that the ingest decided in the meantime stands. The task skips a decided row, the complete endpoint queues nothing for one, and the mint refuses a multipart row that has a verdict with 410 already_uploaded, for example from an agent that restarts after its complete call and before it reports: new part URLs would be for an upload id that S3 no longer has. The cost: assembly_failed is final too. A later completion cannot turn it into verified.

A resume re-signs only the parts the agent names, against the row's own part size, fixed when the upload was created. It is refused when it declares a different size or sha256: those parts are the bytes the row describes, and re-signing against a new geometry would hand out parts that cannot line up with what is already in flight. I found that one reviewing my own first pass, which had no such check. The presented upload_id is checked against the row rather than believed.

Two small changes to shared code the 202 needed: the base POST view grows a response_status, and the turbo_request event posts on any 2xx rather than a literal 200 — otherwise an endpoint that only accepts the work would emit no event at all.

Deployment

Run sweep_turbo_uploads every hour, like cleanup_turbo_machine_job_statuses, on every worker instance if that is simpler: the runs share the work. Without it, a completion that the storage could not answer stays pending until a result arrives, and a result that has arrived already is not sent again.

The AbortIncompleteMultipartUpload lifecycle rule on the bucket is now load-bearing. Zentral drops the parts when an agent reports giving up, but nothing else removes the parts of an upload the agent never reports, and parts are stored and billed until something does. It is in the docs and the changelog, but it is a deployment step and not only code.

Not in this PR

GCS multipart — its XML API is documented as compatible and the design accommodates it, but it is a second server-side implementation through a different client. And the console views to list and download an artifact.

Tests

Full suite green on each commit after the rebase on main: 9342 tests on the storage commit, 9410 on the turbo commit. makemigrations --check agrees with the new 0004 migration. Mutation checks: removing the 5xx translation, re-raising in the task instead of postponing, dropping the sweep's status filter, its SKIP LOCKED, its claim window, or keying it on updated_at, mapping every botocore error to unavailable, or dropping the GCS credential clause each fails the test aimed at it. Each of the six changes for the three writers was removed one at a time, and each time the test aimed at it failed, and only that one. makemigrations --check finds no change at either commit, and the Hugo docs build (--panicOnWarning) passes. Before the rebase: 100% coverage on every line of the three modules this touches (zentral/utils/storage.py, zentral/contrib/turbo/uploads.py, the new tasks.py), verified from coverage json rather than from the diff, since a new file does not show up in a diff-hunk intersection, and 71 new tests.

Every S3 call is stubbed with botocore.stub.Stubber against the real service model, so a parameter S3 does not have — or a response member that is not one — fails here rather than in production. That matters more than usual for this PR: the declaration everything rests on is one S3 accepts silently when it is absent.

[coveralls]

Coverage Report for CI Build 39573539637

Coverage increased (+0.05%) to 91.187%

Details

• Coverage increased (+0.05%) from the base build.
• Patch coverage: 340 of 340 lines across 9 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	59516
Covered Lines:	54271
Line Coverage:	91.19%
Coverage Strength:	0.91 hits per line

---

💛 - Coveralls

[np5]

Read the two commits on the rebased base, which is the reviewed #1685 head. Ran ruff (clean) and mkdocs build --strict (clean). CI on 669940fc was still running when I posted this; the previous head was green.

Because the Stubber tests validate parameters against the service model and not S3's behaviour, I checked the semantics this design rests on against the AWS documentation: part-level checksums are optional when the upload uses a full-object checksum type, so a part that carries only its length is legal; the completion validates the whole-object value and answers BadDigest on a mismatch; an x-amz-mp-object-size mismatch is InvalidRequest, which your terminal set already holds; and the docs state that a completion checksum is accepted but not validated when the algorithm was not declared at creation, which is the "one line that must not be dropped". The control plane is right.

Two things to fix before merge, both in the plumbing rather than the design:

• zentral/contrib/turbo/tasks.py: the completion task saves the whole row minutes after loading it, and the results ingest writes the same row in between. The normal sequence leaves the row pending for good after its result was acknowledged. update_fields on both writers.
• zentral/contrib/turbo/public_views/uploads.py: create_multipart_upload is the one storage call on a device endpoint that nothing wraps, so a storage that does not answer becomes an HTML 500 on the mint.

Smaller: the NoSuchUpload branch can compare the CRC the HEAD returns instead of the size alone; the endpoint reference lacks the multipart response shape and the resume request fields; two unit slips in the docs. The PR description still says both clients pass max_attempts of 0, while the code passes total_max_attempts: 1, which is the right spelling and the one the tests assert.

One question for the "not in this PR" list: after the task's five retries, or a lost enqueue, nothing asks for a completion again except another result or another call from the agent. A periodic sweep of multipart rows still pending would be the backstop, the way the lifecycle rule is for the parts.

What looked good: the threshold argument, the geometry riding the response, completion on the server with the part list taken from the storage, the ambiguity of NoSuchUpload resolved by looking at the object, the resume refusing a changed artifact, and the two client ceilings with the retries spelled as a total.

[np5]

All five addressed in 9ec0e27, and the PR description is corrected — it still claimed both clients pass max_attempts: 0, which was my spelling before the rebase picked up yours.

The lost update was the real one. Your sequence was right, and I only trust the new test because it fails on the old code: reverting the update_fields gives AssertionError: 'pending' != UploadStatus.UPLOADED. Worth noting it was not reachable before this PR — the single-PUT path decides its verification inline, inside the same request that writes the status, so there was no second writer and no window. Multipart introduced both.

On the sweep. You are right that nothing asks again once the five retries are spent or both enqueues are lost, and the row then stays pending for ever while its parts age out on the lifecycle rule — leaving neither an object nor a verdict, and a row that counts against MAX_PENDING_UPLOADS for good.

I have kept it out of this PR and recorded it in the plan, for one reason worth stating: the fix is a periodic re-enqueue, not a verdict on timeout. A storage that was unavailable is not a failed assembly, which #1685 settled for the HEAD path and which holds here for the same reason — so the sweep must not be tempted to write assembly_failed when it gives up. That makes it a small piece of work with a rule of its own, and this app already has the shape for it in cleanup_turbo_machine_job_statuses. Bounded by age, and capped per pass.

Full suite green at 8704, and zero missing lines in every module this touches.

[np5]

Checked 9ec0e27b. All five threads are fixed, and I resolved them.

Verified in the code: the task writes only the verification pair and the ingest only the status axis, both through update_fields, and the new test closes the status axis inside the mocked completion, which is the exact window the docs allow. Starting the multipart upload answers 503 storage_unavailable on a storage that does not answer or refuses, and the half-built row that persists is a normal pending row the retry mint finishes. The NoSuchUpload branch identifies the object by its CRC with the size as fallback. The reference carries the multipart response, the two resume fields and MiB units, and the description now tells the retries story as the code has it.

The test run on this head was still in progress when I checked; the previous head was green. Ruff and the strict docs build are clean.

Nothing open from my side. One note for the list of things not in this PR: a completion that exhausted its retries, or whose enqueue was lost, is only asked for again by another result or another call from the agent. A periodic sweep of multipart rows still pending would be the backstop.

[np5]

Folded the review commit into the two it belonged to, so the branch is back to:

bbcb618e  turbo: multipart, and the server closes it
8a368975  storage: the multipart control plane, and the declaration it rests on

All of it went into the second one — the fixes touched six files, none of them zentral/utils/storage.py or tests/utils/test_storage.py, so there was nothing to split.

The tree is unchanged by the fold: git diff 9ec0e27b bbcb618e is empty, which is the check worth reporting, since a force-push over a branch you have already read is otherwise something you have to take on trust. 849 turbo + utils tests pass on it, and CI is re-running.

The commit message absorbed the reasoning rather than keeping it as a fix, because after folding there was never a bug to fix. Three things earned a place in it: why each writer of the row names the fields it owns (two axes, and a completion that runs for minutes), the 503 on the mint, and the whole-object CRC as what tells the two NoSuchUpload cases apart.

9ec0e27b no longer exists on the branch, so my earlier replies cite a commit you cannot check out. I have noted the surviving SHA on each thread.

[soysauceconspiracy]

LGTM