OK. LGTM then :) On Wed, Sep 13, 2017 at 12:38 PM Mickael Maison <mickael.mai...@gmail.com> wrote:
> Yes exactly ! > I've updated the KIP to make it more explicit. > > Also I noticed my initial email didn't contain the link to the KIP, so > here it is: > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-190%3A+Handle+client-ids+consistently+between+clients+and+brokers > > On Tue, Sep 12, 2017 at 7:23 PM, Gwen Shapira <g...@confluent.io> wrote: > > If I understand you correctly, you are saying: > > 1. KIP-190 will not affect anyone who doesn't use special characters in > > their client IDs > > 2. Those who have special characters in client IDs already have tons of > > metrics issues and won't be inconvenienced by a KIP that fixes them. > > > > Did I get it right? > > > > On Sat, Sep 9, 2017 at 9:54 AM Mickael Maison <mickael.mai...@gmail.com> > > wrote: > > > >> Hi Gwen, thanks for taking a look at the KIP. > >> > >> I understand your concern trying to make the transition as smooth as > >> possible. However there are several issues with the way client-ids > >> with special characters are handled: > >> Client-ids that contain invalid ObjectName characters (colon, equals, > >> etc) currently fail to be registered by the build-in JMX reporter so > >> they already don't appear in all monitoring systems ! These also cause > >> issues with Quotas. > >> > >> The Java clients as well as the kafka-configs.sh tool already reject > >> them (even though the error you get from the Produce/Consumer is > >> pretty cryptic). > >> > >> People currently using client-ids with special characters have to be > >> running 3rd party clients and probably encounter strange quotas issues > >> as well as missing metrics (if they use JMX). > >> > >> So if we really want to do the smallest possible change, we could only > >> encode ObjectName special characters instead of all special > >> characters. That way at least the JMX reporter would work correctly. > >> Display a warning when using any other special characters. Then in a > >> later release, encode everything like we currently do for the > >> User/Principal. > >> > >> What do you think ? > >> > >> On Fri, Sep 1, 2017 at 7:33 AM, Gwen Shapira <g...@confluent.io> wrote: > >> > Thanks for bumping this. I do have a concern: > >> > > >> > This proposal changes the names of existing metrics - as such, it will > >> > require all owners of monitoring systems to update their dashboards. > It > >> > will also complicate monitoring of multiple clusters with different > >> > versions and require some modifications to existing monitoring > automation > >> > systems. > >> > > >> > What do you think of an alternative solution: > >> > 1. For the next release, add the validations on the broker side and > >> print a > >> > "warning" that this client id is invalid, that it will break metrics > and > >> > that it will be rejected in newer versions. > >> > 2. Few releases later, actually turn the validation on and return > >> > InvalidClientID error to clients. > >> > > >> > We did something similar when we deprecated acks=2. > >> > > >> > Gwen > >> > > >> > > >> > On Thu, Aug 31, 2017 at 12:13 PM Mickael Maison < > >> mickael.mai...@gmail.com> > >> > wrote: > >> > > >> >> Even though it's pretty non controversial, I was expecting a few > >> comments. > >> >> I'll wait until next week for comments then I'll start the vote. > >> >> > >> >> Thanks > >> >> > >> >> On Mon, Aug 21, 2017 at 6:51 AM, Mickael Maison > >> >> <mickael.mai...@gmail.com> wrote: > >> >> > Hi all, > >> >> > > >> >> > I have created a KIP to cleanup the way client-ids are handled by > >> >> > brokers and clients. > >> >> > > >> >> > Currently the Java clients have some restrictions on the client-ids > >> >> > that are not enforced by the brokers. Using 3rd party clients, > >> >> > client-ids containing any characters can be used causing some > strange > >> >> > behaviours in the way brokers handle metrics and quotas. > >> >> > > >> >> > Feedback is appreciated. > >> >> > > >> >> > Thanks > >> >> > >> >
