Hi Murali, On Sun, May 3, 2026, at 4:23 PM, Muralidhar Basani via dev wrote: > Hi Kirk, > > Thanks for this thoughtful kip. > > I have a few additional points. > > mb-1 : Would there be any recommendation for reference implementation to > store the client configs ? may be in a compacted topic etc. Otherwise, > everyone will come up with their impl, and could be re-invented again and > again?
This is something I thought about. I didn't want to add a reference implementation that wasn't intended to be fully supported. I would anticipate that any non-trivial implementation would end up either being so specific to a given organization that it's not reusable, or end up being so generalized that it would warrant proper thought (and a KIP) of its own. > mb-2 : When a client reconnects, there might be duplicates of clientconfigs > pushed. Should this be handled by plugin may be based on uuid/clientid? (I > am not sure, if client restarts and there is a new uuid, would there be > stale entries indefinitely) When the client restarts, it is assigned a new client instance ID (see also KIP-1313), and would send configuration again. (The client doesn't know that it's restarted since there's no context. Additionally, there's no way the client can know if the configuration changed during the restart.) Whether or not that's duplicate information is, I guess, a matter of perspective. There is not enough context (either with this KIP or KIP-1313) to maintain history across client invocations. That's a problem we should solve, but it's outside the scope of this KIP. > mb-3 : In your reply to Hector, you noted "it's difficult to ensure that > client is marked as 'in violation' across all brokers in a cluster." Does > the same challenge apply to the observability goal? If the PushConfig lands > on a single randomly chosen broker, only that broker's ClientConfigPolicy > sees it, so the plugin effectively must fan out to shared storage for the > feature to be useful cluster-wide ? Yes, there's an implicit assumption that, like KIP-714, the configuration will end up in a shared system downstream. Outside of a shared topic or hooking into the low level coordination primitives, the mechanisms to spread that information from one broker to the others isn't readily available. > mb-4 : If AdminClient configs are not handled, how about any dynamic config > changes like AdminClient.alterClientConfigs would also work like hot reload > ? Or any explicit re-push mechanism be possible? Eventually we would like to see a hot reload mechanism in the clients, either initiated from the client itself or signaled by the broker. Those are both very useful, but require large areas of exploration. As such it is outside the scope of this KIP :( > mb-5 : Regarding configs.push.allowed.keys, would it help if there is a > deny list which can override allow list? or to avoid any sensitive keys > just in case. Can you provide a use case that would require that? > mb-6 : In general, if the rpc fails for one random node, will it retry to a > different node? Yes, the retries should target a different broker node. Thanks for the feedback! Kirk > > > Thanks, > Murali > > On Sun, May 3, 2026 at 1:37 PM Kirk True <[email protected]> wrote: > > > Hi Andrew, > > > > On Tue, Apr 28, 2026, at 6:28 PM, Andrew Schofield wrote: > > > Hi Kirk, > > > Thanks for the KIP. Inevitably, I have some comments. > > > > > > AS1: I see a few mysterious mentions of "profile". I suppose that these > > are evidence of a concept which did not eventually see the light of day in > > the KIP. > > > > That's a vestige from a previous design. I will remove them ASAP. > > > > > AS2: It seems that the default configurations pushed for a share > > consumer are incorrect (because some of them are group configs not client > > configs). I suggest: > > > > > > * client.id > > > * group.id > > > * share.acknowledgement.mode > > > * share.acquire.mode > > > * max.poll.records > > > * max.poll.interval.ms > > > * fetch.min.bytes > > > * fetch.max.wait.ms > > > > Thanks for the feedback! > > > > > AS3: Is there any reason why you need to send the configuration type > > information to the broker? I wonder if it would be simpler just to leave > > everything as strings. I see that you've got a enum for ClientConfigType > > which is awfully similar to Config.Type, and you're going to end up mapping > > between these enums. > > > > Most of the configuration we're collecting by default end up being > > numeric, so defaulting to strings is less efficient. Also, the thought > > process was that implementors of the ClientConfigPolicy plugin may benefit > > from/need the type information to achieve their goal. > > > > Agreed that the mapping between ClientConfigType and Config.Type seems > > unnecessary. ClientConfigType is a strict subset of Config.Type, > > specifically no PASSWORD type, so I wanted to provide some compile time > > support to prevent their usage. Maybe I was being too zealous in this > > regard? > > > > > AS4: ClientConfigUnknownProfileException has a couple of problems. We > > tend to use Unknown at the start of the error codes and exception class > > names, not the middle. Profile is not a thing. I suggest it should be > > UnknownClientConfigSomethingException, but I am not qualified to say what > > the Something is. > > > > I will propose a change in the next revision of the KIP. Thanks for > > catching that. > > > > > AS5: You should include a list of the exceptions and error codes you are > > introducing to the protocol. I've seen a few new exceptions and they > > generally have error codes which correspond 1:1. > > > > > > * ClientConfigUnknownProfileException - Should this just be > > InvalidConfigurationException (exists) which maps to INVALID_CONFIG error > > code? > > > * ClientConfigTooLargeException - I would expect this to map to the > > CLIENT_CONFIG_TOO_LARGE error code (new) > > > * ClientConfigPolicyException - See AS4. Policy exceptions are usually > > PolicyViolationException, but this policy doesn't validate, it just > > processes. > > > > I wasn't certain where on the specificity vs. generality spectrum to land > > on creating or reusing existing errors. In my next KIP revision I'll look > > to reuse existing error codes where possible. > > > > > AS6: You've tended to use Config (singular) not Configs (plural). > > However, in the configurations for the configurations, you've used plural, > > such as client.configs.policy.class.name. I would err on the side of > > consistency. > > > > Agreed. I'll make this more consistent. > > > > > AS7: We are going to have to resolve the relationship between this KIP > > and KIP-1313. The latter introduces client instance ID on all RPCs, and > > will simplify your KIP if it is accepted first. > > > > Yes, KIP-1313 are racing in this regard. I see KIP-1313 has entered the > > voting phase, so I will likely end up removing most of the client ID > > instance references from my KIP, leaving on those that are more pertinent > > to the KIP's specific needs. > > > > > AS8: Please confirm whether there are any timing considerations between > > PushConfigs and GetTelemetrySubscriptions/PushTelemetry RPCs. I suppose > > that a client could overlap these RPCs, or even send them to different > > randomly selected brokers, as part of its initial connection setup. > > > > The intention was for the PushConfig RPC to execute before the telemetry > > RPCs. However, this will likely change based on other feedback, moving > > toward more of an "overlap" sequencing. From the perspective of this KIP, > > it shouldn't matter if they're sent to different brokers. I wonder if the > > ClientConfigPolicy plugin implementors would see that differently? > > > > > AS9: Please add some broker metrics for this new feature. I suggest > > looking at KIP-714 for inspiration. > > > > Will do. I intentionally omitted metrics on the broker, leaving them for > > the specific implementations. But I will look at the KIP-714 metrics, as > > suggested. > > > > > AS10: Why does the client block while the config handshake is being > > performed? The handshake is not validating the configurations and the > > client doesn't throw an exception even if the PushConfigs response contains > > an error code. Doesn't this unnecessarily slow down the initial connection, > > and that's arguably too long with Kafka clients already. > > > > The approach will likely move away from a blocking approach to a > > background, best effort approach. I will update the KIP to reflect this. > > > > > AS11: Which are the default configurations sent for the Apache Kafka > > Java admin client? > > > > In initial discussions, the admin client was deemed as too uninteresting > > to warrant sending configuration since it doesn't effect correctness or > > performance of the critical produce/consume path. Do you disagree? > > > > Thanks for the thorough feedback! > > > > Kirk > > > > > Thanks, > > > Andrew > > > > > > On 2026/04/23 17:59:11 Kirk True wrote: > > > > Hi all, > > > > > > > > I would like to start a discussion on KIP-1324: Support client > > configuration observability: > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1324%3A+Support+client+configuration+observability > > > > > > > > Thanks, > > > > Kirk > > > > > > > > > >
