vismaytiwari opened a new pull request, #22792:
URL: https://github.com/apache/kafka/pull/22792

   JIRA: https://issues.apache.org/jira/browse/KAFKA-20650
   
   ### Problem
   In combined (KRaft) mode, `BrokerServer` and `ControllerServer` each call 
`QuotaFactory.instantiate()`, which creates a separate `ClientQuotaCallback` 
via reflection per role. A custom callback is therefore instantiated **twice 
with divergent state in one JVM** — a regression from the pre-KRaft 
single-instance behavior.
   
   ### Approach
   Create the callback once in `SharedServer` (which the broker and controller 
already share for `metrics`, `raftManager`, etc.) and pass it into a new 
`QuotaFactory.instantiate(...)` overload that accepts a pre-built 
`Optional<Plugin<ClientQuotaCallback>>`, so both roles use the same instance. 
Separate-mode broker-only / controller-only behavior is unchanged (one role per 
`SharedServer`).
   
   ### Open questions for reviewers (this is why I'm raising it as a discussion)
   1. **Close ownership.** The shared plugin is currently closed by each role's 
`QuotaManagers.shutdown()`, so in combined mode it is closed twice (benign via 
`Utils.closeQuietly`, but not ideal). I can instead track ownership so only 
`SharedServer` closes it, or have `SharedServer` own the close and 
`QuotaManagers` skip it — happy to implement whichever you prefer. I kept this 
PR minimal to align on the approach first rather than churn the `QuotaManagers` 
record + its test call sites.
   2. **Config / role tag.** The shared callback is built from 
`sharedServerConfig` with the `broker` role tag. In combined mode the 
broker/controller configs share the same props, but let me know if you'd prefer 
a different config or a combined role tag.
   
   ### Testing / honesty
   I couldn't run a local build for this (no JDK available in my environment), 
so I've relied on careful manual review and am leaning on CI here — apologies 
if something needs a fixup; I'll iterate quickly. Guidance on the two questions 
above would let me finish it cleanly.
   
   AI disclosure: drafted with Claude (Claude Code) and reviewed; the commit 
carries the `Co-Authored-By` trailer per the contributing guide.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to