feat(s3): conclude parked blobs in chunked single-request batches (#69) - #83
Draft
frrist wants to merge 2 commits into
Draft
feat(s3): conclude parked blobs in chunked single-request batches (#69)#83frrist wants to merge 2 commits into
frrist wants to merge 2 commits into
Conversation
…etry (#69) Deferred-accept multipart concluded parked blobs sequentially at Complete: one /ucan/conclude round trip + receipt fetch per blob held the connection open O(parts) — minutes at a few hundred parts, past any client's fixed read timeout (the AWS CLI's 60 s fires at ~250-500 MiB with default chunk sizes). The retry then landed while the session was latched 'completing', lost the open→completing latch, and was mapped to NoSuchUpload — a terminal 404 for an upload that usually went on to commit. Two fixes, both client-side (the batched-conclude protocol change through libforge/sprue remains follow-up): - concludeBlobs fans out over distinct digests through a bounded errgroup (16 in flight). Sprue answers conclude with the accept receipt already stored, so the phase costs waves of round trips instead of their sum: seconds for 256 parts. A first error cancels the rest; concluded blobs keep their locations and the remainder stay parked (conclude is idempotent), so a retry resumes where the attempt stopped. - A Complete that loses the latch no longer reports NoSuchUpload blindly: it inspects the session and joins an in-flight peer — polling until the winner resolves the row, then replaying its committed result (the part validation already matched the retained parts), taking over if the peer reverted to 'open', and yielding a retryable SlowDown once the join budget (2 min) is spent on a wedged/stranded row. Losing to an Abort still 404s; so does a reaped session. The join promotes CompleteMultipartUpload/racey_data_integrity (five concurrent Completes of one upload id, all required to succeed with the winner's ETag) out of the XFail table: it flagged the fix as an unexpected pass. Verified: unit suite (join/takeover/budget/abort-race + conclude-overlap gate tests, -race), go vet, and the full itest conformance partition against the smelt stack (twice: once flagging the unexpected pass, once green after the promotion). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Complete's accept phase no longer spends one HTTP round trip per parked blob. The UCAN container is already a batch primitive — the stock ucantone server executes EVERY invocation in a request container addressed to it and returns all their receipts in one response container, and the stock client merges WithInvocations into the request it sends — so batching needs no protocol or sprue change: - forgeclient.BlobConcludeBatch synthesizes every parked blob's put receipt and ships all the conclude invocations + receipts in ONE POST, recovers each conclude's receipt from the response container by task link, and fetches the accept receipts (location commitments) bounded-parallel — sprue stores them before answering, so the first fetch succeeds. Outcomes are positional and per-blob; failed entries stay parked and idempotently retryable. A round-trip test drives the real ucantone server + receipt endpoint in-process and pins the one-POST behavior. - uploader.DeferredBodyUploader trades ConcludeBlob for ConcludeBlobBatch; s3frontend.concludeBlobs triages located/parked/never-parked digests, concludes the parked set in chunks of 16 with 4 chunks in flight (the service executes a container's invocations sequentially while the request hangs open, so the chunk bounds hold-open time and inflight chunks restore overlap), and lets every chunk run to completion on failure — each recorded location is one less conclude for the retry's resume. The per-blob receipt fetches and the sequential server-side execution are the next protocol steps (sprue returning conclude results inline; libforge batch ConcludeArguments so sprue can group accepts by provider) — tracked in #69. Verified: unit suite -race (batch-in-one-call, partial-failure resume, and ucantone round-trip tests), go vet, and the full itest conformance partition against the smelt stack (go test -count=1 — the make target's cached rerun was caught and discarded). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Note
Draft — incomplete by design. This is the client-side half of #69 and a working sketch of the better cross-service design, opened to preserve and socialize the work before a ~5-month absence. It is functional against unmodified sprue (rebased onto current
main, unit suite green), but it is not the end state — see "What remains" below before investing review effort in the details.What this does
Complete's accept phase no longer spends one HTTP round trip per parked blob. The UCAN container is already a batch primitive — the stock ucantone server executes every invocation in a request container addressed to it and returns all their receipts in one response container, and the stock client merges
WithInvocationsinto the request it sends — so batching needs no protocol or sprue change:forgeclient.BlobConcludeBatchsynthesizes every parked blob's put receipt and ships all the conclude invocations + receipts in one POST, recovers each conclude's receipt from the response container by task link, and fetches the accept receipts (location commitments) bounded-parallel — sprue stores them before answering, so the first fetch succeeds. Outcomes are positional and per-blob; failed entries stay parked and idempotently retryable. A round-trip test drives the real ucantone server + receipt endpoint in-process and pins the one-POST behavior.uploader.DeferredBodyUploadertradesConcludeBlobforConcludeBlobBatch;s3frontend.concludeBlobstriages located/parked/never-parked digests, concludes the parked set in chunks of 16 with 4 chunks in flight, and lets every chunk run to completion on failure — each recorded location is one less conclude for the retry's resume.What remains — the full design this sketches toward
This PR batches at the transport level against today's protocol. The end state is protocol-level batching plus removal of the real latency floor, which lives in piri, not here. The complete cross-service plan (issue-ready drafts, verified file:line evidence, sequencing) is preserved in the piri handoff ledger, PIRI.md §9 “Batch / single-request conclude” (handoff bundle — ask @hannahhoward). Summary:
ConcludeArguments.Receipts []cid.Cidso sprue can see the whole batch at once. (unfiled — libforge A)ExecuteBatch, and opt-in bounded-concurrency container execution (measure first). (unfiled)Nothing here depends on hilt; its
blob.Abort/blob.Removefloors are already met.Refs
mainpost lock/tagging; one test-only fix for the newregistry.CreateStateparameter inmem.Create.🤖 Generated with Claude Code