Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts multipart session lifecycle handling in s3frontend to ensure the periodic sweeper correctly reaps stale multipart sessions, including sessions stranded in terminal-ish states due to crashes, aligning behavior with the multipart durability/cleanup model.
Changes:
- Updates sweeper logic to treat crash-stranded
completingsessions like aborts (cleanup then reap), and to finish cleanup for strandedabortingsessions. - Introduces a
reapAbortingSessionhelper to centralize “delete session + cleanup blobs” logic for abort-style reaping. - Clarifies
CompleteMultipartUploadcommentary regarding post-commit session state and sweeper behavior.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1193
to
+1210
| func (b *Backend) reapAbortingSession(ctx context.Context, s registry.MultipartSession) bool { | ||
| // Snapshot the parts' blob digests before the cascade delete. | ||
| var digests [][]byte | ||
| if parts, err := b.multipart.ListParts(ctx, s.UploadID); err == nil { | ||
| for _, p := range parts { | ||
| digests = append(digests, p.BlobDigests...) | ||
| } | ||
| } | ||
| if err := b.multipart.DeleteSession(ctx, s.UploadID); err != nil { | ||
| return false | ||
| } | ||
| space, err := b.bucketSpace(ctx, s.Bucket) | ||
| if err != nil { | ||
| return true // bucket gone; spool rows are reapable later | ||
| } | ||
| b.cleanupPartBlobs(ctx, space, s.UploadID, digests) | ||
| return true | ||
| } |
Comment on lines
615
to
622
| // The object is durable. Retain the session (state 'completed') and its | ||
| // parts so a duplicate Complete is idempotent; the sweeper reaps it later. | ||
| // Best-effort: a failed latch leaves the row in 'completing', which the | ||
| // sweeper also treats as terminal after the TTL. | ||
| // sweeper reaps through its abort path after the TTL — harmless here, as | ||
| // every blob is accepted by now, so the cleanup skips them all. | ||
| if _, err := b.multipart.LatchSession(ctx, uploadID, registry.SessionCompleting, registry.SessionCompleted); err != nil { | ||
| b.logger.Warn("latch session to completed failed; sweeper reaps the completing row after the TTL", | ||
| zap.String("uploadID", uploadID), zap.Error(err)) |
This was referenced Sep 1, 2026
alanshaw
added a commit
that referenced
this pull request
Sep 10, 2026
…mplete replay (#121) An overwrite or delete shredded the superseded object's key and location rows right after the root swap, while an in-flight GET on the prior root still needed them. Two writers racing on one unversioned key could also release a blob the surviving generation still referenced, because every unversioned generation shared one `blob_refs` row. And concurrent `CompleteMultipartUpload` calls returned `NoSuchUpload` to every caller but the latch winner. ## Changes **Deferred release.** Dropping a blob's last claim records a release intent in the same transaction instead of releasing inline. A sweeper executes intents after a grace window: it re-checks the claim count, then deletes the encryption-params row, the location row, and the network registration. A failure keeps the intent for the next tick. Claims can exceed one per digest because `CopyObject` is metadata-only: the destination version pins the source's blobs and adds its own claims. PUTs never dedup (fresh CEK per blob), so copies are the only source of shared digests. The sweeper's claim recheck is what protects a copy racing a delete of its source. **Per-generation claims.** `blob_refs.version_id` is `null#<seq>` for unversioned generations, so two generations of one key never share a row. New-generation claims are added under the bucket commit lock before the root swap; superseded claims drop after the commit is durable. **Complete replay.** The winner records its committed ETag and version id on the session. Latch losers wait for the winner's terminal state and replay that recorded result, so a later overwrite of the key cannot disturb the replay. `NoSuchUpload` is returned only when the session is aborting or gone. A loser that outwaits its budget returns `OperationAborted`, which the client retries. **DeleteBucket drains first.** The space's pending releases run immediately (an empty bucket owes no reader grace) before hilt is asked to delete the space, which refuses while blobs remain registered. ## Config and schema `release_grace` (duration, default 60s, negative means immediate). Sweeper interval is `grace/2`, clamped to [1s, 10min]. Migration `00015_blob_release_intents.sql` adds `ingot.blob_release_intents (space, digest, not_before, created_at)`; `00016_multipart_committed_result.sql` adds `committed_etag` and `committed_version_id` to `ingot.multipart_sessions`. ## Testing Unit: concurrent Completes share one ETag; the DeleteBucket drain ignores grace; refindex and multipart-hygiene assertions drain before checking. Live Postgres: atomic drop-and-enqueue. Integration: new `OverwriteRace` (readers see exactly the old or the new object, never a mix); `racey_data_integrity` moves from XFail to pass. Supersedes #90.
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.
Fixes the issue identified in fil-one/RFC#15 (comment)