feat: KafkaProxy threading and lifecycle contract#98
Conversation
Define the single-use lifecycle and threading contract for KafkaProxy, including idempotent startup/shutdown, the future-returning startup() API, and deprecation of block(). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sbarker@redhat.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
017 and 018 were claimed by PR kroxylicious#97. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sbarker@redhat.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
- Note that KafkaProxy is not public API - Clarify sidecar/embedded are not supported deployment models today - Document exceptional completion of the startup future - Add process exit code contract for the two edges into STOPPED - Add metrics section (state gauge, startup duration) - Update rejected alternatives to reference metrics Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
f747944 to
9964028
Compare
tombentley
left a comment
There was a problem hiding this comment.
Thanks Sam! I think the proposal is becoming clearer to me now, and it makes sense. I've made a few suggestions for how it's being explained.
- Replace "implementors" with "Kroxylicious developers" - Reword "exception propagates" to "returning a failed future" Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
k-wall
left a comment
There was a problem hiding this comment.
LGTM - thanks for formalizing this.
Add PRs kroxylicious#96, kroxylicious#98, kroxylicious#99, kroxylicious#100, kroxylicious#101, and kroxylicious#103 which were opened after the initial script was created. Note: PR kroxylicious#100 is already correctly named (100-sidecar-injection-webhook.md). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This change simplifies the proposal numbering system by using PR numbers as proposal identifiers, eliminating number collisions and removing the need for a separate allocation process. Changes: - Simplified proposals/README.md to focus on author workflow - Removed index tables (directory listing serves as the index) - Streamlined instructions for creating and renaming proposals - Updated proposal template with workflow instructions - Require PR number in title format: # <PR#> - <Title> - Moved workflow instructions into comment block - Added GitHub workflow to automatically check proposal numbering - Validates both filename and title format - Updates PR description when proposal files don't match PR number - Provides exact commands to fix naming issues - Removes warning once corrected - Handles both added and renamed files - Runs on all PRs (ready for mandatory status check) - Added notification script for existing open PRs - After merge, run notify-open-prs.sh to ask authors to rebase - Workflow will automatically guide them through renaming - Updated with all current open proposal PRs (kroxylicious#70, kroxylicious#82, kroxylicious#83, kroxylicious#85, kroxylicious#88, kroxylicious#93, kroxylicious#94, kroxylicious#96, kroxylicious#98, kroxylicious#99, kroxylicious#100, kroxylicious#101, kroxylicious#103) Proposals 001-019 retain their original numbers. Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>
This change simplifies the proposal numbering system by using PR numbers as proposal identifiers, eliminating number collisions and removing the need for a separate allocation process. Changes: - Simplified proposals/README.md to focus on author workflow - Removed index tables (directory listing serves as the index) - Streamlined instructions for creating and renaming proposals - Updated proposal template with workflow instructions - Require PR number in title format: # <PR#> - <Title> - Moved workflow instructions into comment block - Added GitHub workflow to automatically check proposal numbering - Validates both filename and title format - Updates PR description when proposal files don't match PR number - Provides exact commands to fix naming issues - Removes warning once corrected - Handles both added and renamed files - Runs on all PRs (ready for mandatory status check) - Added notification script for existing open PRs - After merge, run notify-open-prs.sh to ask authors to rebase - Workflow will automatically guide them through renaming - Updated with all current open proposal PRs (#70, #82, #83, #85, #88, #93, #94, #96, #98, #99, #100, #101, #103) Proposals 001-019 retain their original numbers. Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>
|
Hi! We've updated the proposal numbering system to use PR numbers as proposal identifiers. Action required: Please rebase your PR on Once you rebase, you'll need to rename your proposal file and update the title: git mv proposals/019-kafkaproxy-threading-and-lifecycle.md proposals/098-threading-and-lifecycle.md
# Update title: remove any old number prefix and add PR number
sed -i.bak '0,/^# /{s/^# \([0-9]\{3\}\|xxx\|nnn\|000\) - /# 98 - /; t; s/^# /# 98 - /}' proposals/098-threading-and-lifecycle.md && rm proposals/098-threading-and-lifecycle.md.bak
git add proposals/098-threading-and-lifecycle.md
git commit -m "Rename proposal to use PR number"
git pushThe GitHub workflow will automatically check your proposal file naming after you push and update this PR description if any corrections are still needed. See proposals/README.md for the updated workflow. |
|
Correction: The previous notification had an incorrect filename. Here are the correct commands: git mv proposals/019-kafkaproxy-threading-and-lifecycle.md proposals/098-kafkaproxy-threading-and-lifecycle.md
# Update title: remove any old number prefix and add PR number
sed -i.bak '0,/^# /{s/^# \([0-9]\{3\}\|xxx\|nnn\|000\) - /# 98 - /; t; s/^# /# 98 - /}' proposals/098-kafkaproxy-threading-and-lifecycle.md && rm proposals/098-kafkaproxy-threading-and-lifecycle.md.bak
git add proposals/098-kafkaproxy-threading-and-lifecycle.md
git commit -m "Rename proposal to use PR number"
git pushSorry for the confusion! |
- Rename 'startup() Returns a Future' section to 'The Lifecycle Future' - Lead with the key concept: one future represents the full proxy lifetime - Add three-outcome table (startup failure, clean shutdown, shutdown failure) - Document cancel() semantics: equivalent to shutdown(), flag ignored - Update concurrency guarantee kroxylicious#3 to reinforce the single-future model - Add shutdown failure as a third path in Process Exit Codes - Note that startup() return type changes from KafkaProxy to CompletableFuture<Void> - Remove Kubernetes-specific kube_pod_status_phase reference from Metrics - Add 'Separate futures per phase' rejected alternative - Add 'Hard shutdown via cancel(true)' rejected alternative - Fix stray typo in Motivation section Assisted-by: Claude claude-sonnet-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
k-wall
left a comment
There was a problem hiding this comment.
Couple of nits but other LGTM.
Thanks for hardening this contract up.
- Fix proposal number in title (019 -> 98) - Fix wording: "before the startup future is failed exceptionally" - Remove block() — internal method, no external callers, join() is the replacement Signed-off-by: Sam Barker <sbarker@redhat.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Co-authored-by: Tom Bentley <tombentley@users.noreply.github.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Summary
Define the threading and lifecycle contract for
KafkaProxy: single-use, idempotentstartup()/shutdown(), future-returningstartup()API, deprecation ofblock().This is a prerequisite for the virtual cluster lifecycle work in 016 — defining the outer proxy contract first so that inner components can make informed concurrency decisions.
Key decisions
KafkaProxyis single-use:NEW→STARTING→STARTED→STOPPING→STOPPEDstartup()returns aCompletableFuture<Void>that completes when the proxy stops — replacesblock()startup()andshutdown()are idempotent — callers do not need to coordinateshutdown()may be called from any thread (JVM shutdown hook)startup()afterSTOPPING/STOPPED(not restartable)FAILEDstate at proxy level — failure propagates via exception, proxy is not reconfigurableTest plan
🤖 Generated with Claude Code