Add S3 PUT admission control by bootjp · Pull Request #1000 · bootjp/elastickv

bootjp · 2026-07-07T05:56:25Z

Summary

add S3 PUT admission control with bounded in-flight body accounting
expose admission wait/rejection/inflight metrics
wire admission control through the S3 server startup path and mark the design implemented

Design doc

docs/design/2026_04_25_implemented_s3_admission_control.md

Validation

go test . ./adapter ./monitoring -run 'TestS3.*Admission|TestStartS3|TestRegistryReturnsS3|TestS3PutAdmission|TestWithS3PutAdmission' -count=1
golangci-lint --config=.golangci.yaml run . ./adapter ./monitoring --timeout=5m
git verify-commit HEAD

Author: bootjp

coderabbitai · 2026-07-07T05:56:33Z

Warning

Review limit reached

@bootjp, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72566b9c-8d9d-4c68-bdbc-1d5e19dc9869

📥 Commits

Reviewing files that changed from the base of the PR and between e8ba43c and 74f4247.

📒 Files selected for processing (14)

adapter/s3.go
adapter/s3_admission.go
adapter/s3_admission_test.go
adapter/s3_chunked.go
adapter/s3_test.go
docs/design/2026_04_25_implemented_s3_admission_control.md
docs/design/2026_04_25_proposed_s3_raft_blob_offload.md
docs/design/2026_06_23_proposed_scaling_roadmap.md
main.go
main_s3.go
main_s3_test.go
monitoring/registry.go
monitoring/s3.go
monitoring/s3_test.go

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

gemini-code-assist

Code Review

This pull request implements S3 PUT admission control to manage aggregate in-flight memory, introducing a per-node semaphore and wiring it into the S3 PutObject and multipart UploadPart paths, alongside Prometheus metrics for monitoring. Feedback from the review highlights several key improvements: resolving a potential deadlock risk in unit acquisition by restricting it to single-unit operations, avoiding a typed nil interface issue in the metrics observer registry, and simplifying the Prometheus metrics by using a standard prometheus.Gauge instead of a vector for label-less metrics.

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.

bootjp · 2026-07-07T12:31:45Z

@codex review

chatgpt-codex-connector

💡 Codex Review

elastickv/docs/design/2026_04_25_implemented_s3_admission_control.md

Lines 440 to 441 in 08f8f3c

    
           `ELASTICKV_S3_PUT_ADMISSION_MAX_INFLIGHT_BYTES=$((1<<63))` to 
        
           disable the cap on M1 nodes during the burn-in window if desired.

Document a parseable admission-disable value

This burn-in instruction does not actually disable the cap in typical shells: $((1<<63)) expands to the signed value -9223372036854775808, and parseS3AdmissionInt64Env treats non-positive values as invalid and falls back to the 256 MiB default. Operators following this during a rollout would unexpectedly keep admission enabled and could still see SlowDown responses.

ℹ️ 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".

bootjp · 2026-07-07T22:34:53Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a73046d2fe

ℹ️ 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".

bootjp · 2026-07-08T01:06:15Z

@codex review

対応しました。

固定長 PUT/UploadPart の admission 事前判定を次のチャンク分に限定し、S3 上限超過だけを EntityTooLarge として扱うようにしました。
body read 後に n>0 の場合だけ admission slot を取得するようにし、EOF 確認だけでは slot を消費しないようにしました。
pending admission が cap を埋める前に batch を flush し、単独アップロードが admission cap をオブジェクトサイズ上限として扱わないようにしました。

検証:

go test ./adapter ./monitoring -run 'TestS3|TestS3PutAdmission|TestS3Server_PutObjectAdmission|TestS3Server_UploadPartAdmission|TestS3Server_UploadPart|TestS3Metrics|TestRegistryReturnsS3PutAdmissionObserver' -count=1 -timeout=240s\n- git diff --check\n- git verify-commit HEAD

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a6ff2b822

ℹ️ 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".

bootjp · 2026-07-08T01:44:48Z

S3 admission/body-read review fixes pushed.

Removed the admission timeout from request-body reads; the timeout now only bounds admission acquisition.
Reworked S3 PUT/UploadPart chunk reads so aws-chunked decoder errors are preserved, including full-buffer reads with an error.
Validated aws-chunked X-Amz-Decoded-Content-Length before admission headroom so S3 size-limit errors return EntityTooLarge instead of SlowDown.

Validation:

go test ./adapter -run 'TestS3PutAdmission|TestS3Server_PutObjectAdmission|TestS3Server_UploadPartAdmission|TestS3Server_PutObjectStreaming|TestS3Server_UploadPartStreaming|TestReadS3PutChunk|TestS3Server_UploadPart|TestS3Server_PutObjectRejectsTruncated' -count=1 -timeout=240s\n- go test ./adapter -run '^TestS3' -count=1 -timeout=240s\n- git diff --check\n- git verify-commit HEAD\n\n@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de31938b4c

ℹ️ 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".

bootjp · 2026-07-08T02:42:03Z

@codex review

対応しました。

PutObject/UploadPart の aws-chunked protocol validation を admission headroom check より前に移し、budget 状態で NotImplemented/InvalidRequest が SlowDown/MissingContentLength に潰れないようにしました。
固定長 body の最終 read が full buffer + io.EOF を返すケースを正常 EOF として扱い、中間 chunk の EOF は IncompleteBody のままにしました。
non-EOF body read error は per-batch admission 取得より前に分類し、破棄する bytes に対して SlowDown を返さないようにしました。

確認:

golangci-lint --config=.golangci.yaml run --fix ./adapter/...
go test ./adapter -run 'TestReadS3PutChunk|TestS3Server_PutObjectAdmission|TestS3Server_UploadPartAdmission|TestS3Server_PutObjectStreamingRejects|TestS3Server_UploadPartStreamingRejects|TestS3Server_PutObjectRejectsAwsChunkedWithoutStreamingMarker|TestS3Server_PutObjectRejectsTruncatedFixedLengthBody|TestS3Server_UploadPartRejectsTruncatedFixedLengthBody' -count=1 -timeout=240s\n- go test ./adapter -run '^$' -count=1 -timeout=120s\n\n補足: go test ./adapter -count=1 -timeout=300s は広い統合テスト群で 300s timeout しました。対象 S3 admission/streaming テストと compile-only は通っています。

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1df92444d2

ℹ️ 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".

bootjp · 2026-07-08T03:32:39Z

S3 admission review fixes pushed.

Reserved a per-chunk admission slot before PutObject/UploadPart body reads so concurrent readers cannot buffer outside the admission cap.
Released the pre-read slot immediately on EOF/error/zero-byte reads; aws-chunked trailer/EOF probes after the declared decoded length do not reserve a payload slot.
Allowed Content-Length-less plain PUT/UploadPart bodies when the cap is disabled, preserving EOF as a normal final partial read.
Decoupled SlowDown Retry-After from the admission wait timeout and pinned it to a short retry hint.

Validation:

go test ./adapter -run 'TestS3PutAdmission|TestS3Server_PutObjectAdmission|TestS3Server_UploadPartAdmission|TestS3Server_PutObjectStreaming|TestS3Server_UploadPartStreaming|TestReadS3PutChunk|TestS3Server_PutObjectRejectsTruncated|TestS3Server_UploadPartRejectsTruncated' -count=1 -timeout=240s\n- go test ./adapter -run '^TestS3' -count=1 -timeout=240s\n- go test ./adapter -run '^$' -count=1 -timeout=120s\n- golangci-lint --config=.golangci.yaml run ./adapter/... --timeout=5m\n- git diff --check\n- git verify-commit HEAD\n\n@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a15c8d8099

ℹ️ 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".

bootjp · 2026-07-08T04:13:35Z

Fixed the latest fixed-length body read finding.

Changes:

map io.ErrUnexpectedEOF from fixed-length PutObject/UploadPart body reads to IncompleteBody instead of falling through to internal error
keep awsChunkedError classification before the generic unexpected-EOF path so chunked framing errors still return InvalidRequest
add PutObject and UploadPart regression coverage using a reader that returns io.ErrUnexpectedEOF

Caller audit:

classifyS3BodyReadErr is only called from the PutObject and UploadPart read loops.

Validation:

go test ./adapter -run 'TestS3Server_(PutObjectRejectsUnexpectedEOFFixedLengthBody|PutObjectRejectsTruncatedFixedLengthBody|PutObjectAdmissionReportsReadErrorBeforeSlowDown|UploadPartRejectsUnexpectedEOFFixedLengthBody|UploadPartRejectsTruncatedFixedLengthBody|UploadPartAdmissionReportsReadErrorBeforeSlowDown)$' -count=1 -timeout=240s
go test ./adapter -run '^TestS3' -count=1 -timeout=300s
golangci-lint --config=.golangci.yaml run ./adapter/... --timeout=5m
git diff --check
git verify-commit HEAD

Note: go test ./adapter -count=1 -timeout=300s timed out in broader raft/SQS adapter coverage after the focused S3 checks passed.

@codex review

chatgpt-codex-connector · 2026-07-08T04:19:39Z

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: b050baf132

ℹ️ 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".

bootjp · 2026-07-08T20:06:49Z

Ready for review after latest HEAD recheck.

Validation:

go test ./adapter -run '^TestS3' -count=1 -timeout=300s\n- go test ./monitoring . -run 'TestS3|TestRegistryReturnsS3PutAdmissionObserver|TestS3ServerOption' -count=1 -timeout=180s\n- golangci-lint --config=.golangci.yaml run ./adapter/... ./monitoring/... --timeout=5m\n- git diff --check\n- git verify-commit HEAD\n\nCurrent status:\n- HEAD: b050baf1325fec50de97c18efb71ac0716426cf2\n- GitHub checks on latest HEAD: all green\n- GraphQL reviewThreads: all resolved; no non-outdated blocker on latest HEAD\n\n@codex review\n@coderabbitai review

coderabbitai · 2026-07-08T20:07:02Z

@bootjp Thanks for the detailed update and validation summary. I'll take a fresh look at the latest HEAD now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…on-control # Conflicts: # adapter/s3.go # docs/design/2026_06_23_proposed_scaling_roadmap.md # main.go

bootjp · 2026-07-08T20:12:46Z

main 追従で merge conflict を解消しました。

変更:

origin/main を merge し、adapter/s3.go の keyviz label wiring と S3 PUT admission 初期化を両立しました。
main.go/main_s3.go の分割済み startup path に S3PutAdmissionObserver を渡すよう統合しました。
scaling roadmap の実装済み設計文書リンクを main 側の rename と整合させました。

Caller audit:

startS3Server は main_s3_test.go の 2 件。
prepareS3Server は startS3Server からのみ。
newS3Server は prepareS3Server と runtimeServerRunner.prepareAdminForwardServers からのみ。

Validation:

go test ./adapter -run '^TestS3' -count=1 -timeout=300s\n- go test ./monitoring . -run 'TestS3|TestRegistryReturnsS3PutAdmissionObserver|TestStartS3Server|TestLoadS3StaticCredentials' -count=1 -timeout=300s\n- go test ./... -run '^$' -count=1 -timeout=300s\n- golangci-lint --config=.golangci.yaml run ./adapter/... ./monitoring/... . --timeout=5m\n- git diff --check\n- git verify-commit HEAD\n\nHEAD: 74f4247aad195f7b47a986ed9580e4df24716436\n\n@codex review

chatgpt-codex-connector · 2026-07-08T20:23:59Z

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 74f4247aad

ℹ️ 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".

adapter: add S3 PUT admission control

c7929f2

gemini-code-assist Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread adapter/s3_admission.go

Comment thread monitoring/registry.go

Comment thread monitoring/s3.go

Comment thread monitoring/s3.go

Comment thread monitoring/s3.go

Harden S3 put admission accounting

08f8f3c

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3_admission.go

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3_admission.go Outdated

Fix S3 admission body handling

a73046d

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3_admission.go

s3: stream PUT admission batches

0a6ff2b

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3_admission.go Outdated

s3: preserve streaming body errors

de31938

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3.go Outdated

Comment thread adapter/s3.go Outdated

s3: preserve protocol errors before admission

1df9244

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread adapter/s3.go

Comment thread adapter/s3.go

Comment thread adapter/s3_admission.go

s3: reserve admission before body reads

a15c8d8

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread adapter/s3.go Outdated

s3: map short fixed reads to incomplete body

b050baf

bootjp marked this pull request as ready for review July 8, 2026 20:06

Merge remote-tracking branch 'origin/main' into design/s3-put-admissi…

74f4247

…on-control # Conflicts: # adapter/s3.go # docs/design/2026_06_23_proposed_scaling_roadmap.md # main.go

bootjp merged commit b1c0ef9 into main Jul 10, 2026
11 checks passed

bootjp deleted the design/s3-put-admission-control branch July 10, 2026 14:54

	`ELASTICKV_S3_PUT_ADMISSION_MAX_INFLIGHT_BYTES=$((1<<63))` to
	disable the cap on M1 nodes during the burn-in window if desired.

Conversation

bootjp commented Jul 7, 2026

Summary

Design doc

Validation

Uh oh!

coderabbitai Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 8, 2026

Uh oh!

bootjp commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading