Add asynchronous S3 chunk blob backfill - #1126
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bootjp
commented
Jul 19, 2026
Signed head @codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces an asynchronous S3 blob backfiller (S3BlobBackfiller) to handle follower catch-up work independently from the Raft apply loop, complete with background workers, periodic scanning, token-bucket rate limiting, and Prometheus metrics. The feedback highlights three key areas for improvement: ensuring that stopping the backfiller properly drains the queue and clears pending keys to prevent permanent blocks, optimizing the local blob existence check to avoid loading entire payloads into memory, and addressing a potential thundering herd issue in the custom token bucket implementation by adopting a stateful reservation mechanism.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a1d6a3aca6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
a1d6a3a to
100acd3Comparebootjp
commented
Jul 19, 2026
Signed head 100acd3 addresses every current review finding. Changes:
Caller audit:
Verification:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:100acd365f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
100acd3 to
07b963fCompare0a7eed9 to
02e3a6dCompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:07b963fedc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
07b963f to
b374525Comparebootjp
commented
Jul 19, 2026
History rebuilt without content changes and restacked on the sanitized #1063 head.
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b374525c52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
02e3a6d to
77ea547Compareb374525 to
eaaa08bComparebootjp
commented
Jul 19, 2026
Current signed head Changes:
Caller audit:
Verification:
@codex review |
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Uh oh!
There was an error while loading. Please reload this page.
This document declares itself the canonical ownership index, so rows that call finished work unimplemented and unowned send contributors off to write a competing design. Seven rows named work that is already an ancestor of this commit: - route catalog delta watch (#1117, 6c7a66e) -> implemented owner - SST ingest snapshot transfer (#1130, 910a97e) -> implemented owner - leader-proxy circuit breaker (#1132, 56e36e9) -> implemented owner - physical snapshot offload (#1131, 764db2d) -> partial owner, which records M0/M1 implemented and still owns M2/M3 - TSO ceiling state machine (#1150, 0e85c82) -> merged, separated from the group-0 issuance work that remains open - autosplit committed-window reader (#1152, afec059) -> M3 is partially implemented, not absent - S3 offload transport and backfill (77ea547, #1126/5eaaa05d) -> the transport path is merged; the focused design names reference counting, GC readiness, and legacy migration as the remaining blockers Each cited commit was confirmed to be an ancestor of this revision, and every referenced design document was confirmed to exist. The completion rule also could not be satisfied by every row: it demanded each implemented row's focused owner be promoted to `implemented`, but the shared Pebble cache row names only PR #1082 as its canonical owner and a pull request has no status to promote. That made the roadmap permanently ineligible for promotion regardless of what shipped. Merged implementation evidence now closes such a row; an open pull request still does not. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE
Summary
Safety
Verification
go test ./... -count=1go test -race ./adapter -run 'TestS3BlobBackfill|TestS3BlobTokenBucket' -count=3\n-golangci-lint run ./... --timeout=5m --allow-parallel-runners -j 2\n- focused backfill, shard-store, and metrics tests x10\n-git diff --check\n\nStacked on s3: add blob fetch rpc #1063.