kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin McCabe <cmcc...@apache.org>
Subject Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient
Date Fri, 18 May 2018 23:01:40 GMT
Hi Viktor,

Thanks, this looks good.

The boolean should default to false if not set, to ensure that existing clients continue to work as-is, right?  Might be good to add a note specifying that.

+1 (non-binding)

best,
Colin

On Fri, May 18, 2018, at 08:16, Viktor Somogyi wrote:
> Updated KIP-248:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient
> 
> I'd like to ask project members, committers and contributors to vote
> as this would be a useful improvement in Kafka.
> 
> Sections changed:
> - Public interfaces: added the bin/scram-credentials.sh command that
> we discussed with Colin.
> - Wire format types: removed AlterOperations. As discussed with Colin,
> we don't actually need this: we should behave in an incremental way in
> AlterQuotas. For AlterConfig we'll implement this behavior with an
> extra flag on the protocol (and incrementing the version).
> - AlterQuotas protocol: removed AlterOperations. Added some more
> description to the behavior of the protocol. Removing quotas will
> happen by sending a NaN instead of the AlterOperations. (Since IEEE
> 754 covers NaNs and it is not a valid config for any quota, I think it
> is a good notation.)
> - SCRAM: so it will be done by the scram-credentials command that uses
> direct zookeeper connection. I think further modes, like doing it
> through the broker is not necessary. The idea here is that zookeeper
> in this case acts as a credentials store. This should be decoupled
> from the broker as we manage broker credentials as well. The new
> command acts as a client to the store.
> - AlterConfigs will have an incremental_update flag as discussed. By
> default it is false to provide the backward compatible behavior. When
> it is true it will merge the configs with what's there in the node.
> Deletion in incremental mode is done by sending an empty string as
> config value.
> - Other compatibility changes: this KIP doesn't scope listing multiple
> users and client's quotas. As per a conversation with Rajini, it is
> not a common use case and we can add it back later if it is needed. If
> this functionality is needed, the old code should be still available
> through run-kafka-class. (Removed the USE_OLD_KAFKA_CONFIG_COMMAND as
> it doesn't make sense anymore.)
> 
> On Fri, May 18, 2018 at 12:33 PM, Viktor Somogyi
> <viktorsomogyi@gmail.com> wrote:
> > Ok, ignore my previous mail (except the last sentence), gmail didn't
> > update me about your last email :/.
> >
> >> I think we should probably just create a flag for alterConfigs which marks it as incremental, like we discussed earlier, and do this as a compatible change that is needed for the shell command.
> >
> > Alright, I missed that about the sensitive configs too, so in this
> > case I can agree with this. I'll update the KIP this afternoon and
> > update this thread.
> > Thanks again for your contribution.
> >
> > Viktor
> >
> > On Fri, May 18, 2018 at 2:34 AM, Colin McCabe <cmccabe@apache.org> wrote:
> >> Actually, I just realized that this won't work.  The AlterConfigs API is kind of broken right now.  DescribeConfigs won't return the "sensitive" configurations like passwords.  So doing describe + edit + alter will wipe out all sensitive configs. :(
> >>
> >> I think we should probably just create a flag for alterConfigs which marks it as incremental, like we discussed earlier, and do this as a compatible change that is needed for the shell command.
> >>
> >> best,
> >> Colin
> >>
> >>
> >> On Thu, May 17, 2018, at 09:32, Colin McCabe wrote:
> >>> Hi Viktor,
> >>>
> >>> Since the KIP freeze is coming up really soon, maybe we should just drop
> >>> the section about changes to AlterConfigs from KIP-248.  We don't really
> >>> need it here, since ConfigCommand can use AlterConfigs as-is.
> >>>
> >>> We can pick up the discussion about improving AlterConfigs in a future KIP.
> >>>
> >>> cheers,
> >>> Colin
> >>>
> >>> On Wed, May 16, 2018, at 22:06, Colin McCabe wrote:
> >>> > Hi Viktor,
> >>> >
> >>> > The shell command isn’t that easy to integrate into applications.
> >>> > AdminClient will get integrated  into a lot more stuff, which
> >>> > increases the potential for conflicts.  I would argue that we should
> >>> > fix this soon.
> >>> > If we do want to reduce the scope in this KIP, we could do the merge in
> >>> > the ConfigCommand  tool for now, and leave AC unchanged.
> >>> > Best,
> >>> > Colin
> >>> >
> >>> >
> >>> > On Wed, May 16, 2018, at 04:57, Viktor Somogyi wrote:
> >>> > > Hi Colin,
> >>> > >
> >>> > > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> >>> > > > set at the same time as you, you might overwrite that person's
> >>> > > > changes, or vice versa.  So I really don't think we should try to do
> >>> > > > this.  Also, having both an incremental and a full API is useful,
> >>> > > > and it's just a single boolean at the protocol and API level.>
> >>> > > Overwriting somebody's change is currently possible with the
> >>> > > ConfigCommand, as it will do this get-merge-set behavior on the client> side, in the command. From this perspective I think it's not much
> >>> > > different to do this with the admin client. Also I think admins don't> modify the quotas/configs of a client/user/topic/broker often (and
> >>> > > multiple admins would do it even more rarely), so I don't think it is> a big issue. What I think would be useful here but may be out of scope> is to version the changes similarly to leader epochs. So when an admin> updates the configs, it will increment a version number and won't let> other admins to push changes in with lower than that. Instead it would> return an error.
> >>> > >
> >>> > > I would be also interested what others think about this?
> >>> > >
> >>> > > Cheers,
> >>> > > Viktor
> >>> > >
> >>> > >
> >>> > > On Sat, May 12, 2018 at 2:29 AM, Colin McCabe
> >>> > > <cmccabe@apache.org> wrote:> > On Wed, May 9, 2018, at 05:41, Viktor Somogyi wrote:
> >>> > > >> Hi Colin,
> >>> > > >>
> >>> > > >> > We are going to need to create a new version of
> >>> > > >> > AlterConfigsRequest to add the "incremental" boolean.  So while
> >>> > > >> > we're doing that, maybe we can change the type to
> >>> > > >> > NULLABLE_STRING.> >>
> >>> > > >> I was just talking to a colleague yesterday and we came to the
> >>> > > >> conclusion that we should keep the boolean flag only on the client> >> side (as you may have suggested earlier?) and not make part of the> >> protocol as it might lead to a very complicated API on the long
> >>> > > >> term.> >> Also we would keep the server side API simpler. Instead of the
> >>> > > >> protocol change we could just simply have the boolean flag in
> >>> > > >> AlterConfigOptions and the AdminClient should do the get-merge-set> >> logic which corresponds to the behavior of the current
> >>> > > >> ConfigCommand.> >> That way we won't need to change the protocol for now but
> >>> > > >> still have> >> both functionality. What do you think?
> >>> > > >
> >>> > > >  Hi Viktor,
> >>> > > >
> >>> > > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> >>> > > > set at the same time as you, you might overwrite that person's
> >>> > > > changes, or vice versa.  So I really don't think we should try to do
> >>> > > > this.  Also, having both an incremental and a full API is useful,
> >>> > > > and it's just a single boolean at the protocol and API level.> >
> >>> > > >>
> >>> > > >> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or
> >>> > > >> > "<default>" to indicate defaults, does it?> >>
> >>> > > >> No it doesn't. It was just my early idea to indicate "delete"
> >>> > > >> on the> >> protocol level. (We are using <default> for denoting the default
> >>> > > >> client id or user in zookeeper.) Rajini was referring that we
> >>> > > >> shouldn't expose this to the protocol level but instead denote
> >>> > > >> delete> >> with an empty string.
> >>> > > >>
> >>> > > >> > This comes from DescribeConfigsResponse.
> >>> > > >> > Unless I'm missing something, I think your suggestion to not
> >>> > > >> > expose "<default>" is already implemented?> >>
> >>> > > >> In some way, yes. Although this one is used in describe and not in> >> alter. For alter I don't think we'd need my early "<default>" idea.> >
> >>> > > > OK.  Thanks for the explanation.  Using an empty string to indicate
> >>> > > > delete, as Rajini suggested, seems pretty reasonable to me.  null
> >>> > > > would work as well.> >
> >>> > > >>
> >>> > > >> >> And we use STRING rather than NULLABLE_STRING in describe
> >>> > > >> >> configs etc. So we> >> >> should probably do the same for quotas."
> >>> > > >> >
> >>> > > >> > I think nearly all responses treat ERROR_MESSAGE as a nullable
> >>> > > >> > string.  CommonFields#ERROR_MESSAGE, which is used by most of
> >>> > > >> > them, is a nullable string.  It's DescribeConfigsResponse that is
> >>> > > >> > the black sheep here.> >> >
> >>> > > >> >  >     public static final Field.NullableStr ERROR_MESSAGE = new
> >>> > > >> >  >     Field.NullableStr("error_message", "Response error
> >>> > > >> >  >     message");> >>
> >>> > > >> Looking at DescribeConfigsResponse (and AlterConfigsResponse)
> >>> > > >> they use> >> nullable_string in the code. KIP-133 states otherwise though. So in> >> this case it's not a problem luckily.
> >>> > > >
> >>> > > > Thanks for finding this inconsistency.  I'll change the KIP to
> >>> > > > reflect what was actually implemented (nullable string for error).> >
> >>> > > > cheers,
> >>> > > > Colin
> >>> > > >
> >>> > > >>
> >>> > > >> > What about writing a small script that just handles setting up
> >>> > > >> > SCRAM credentials?  It would probably be easier to maintain than
> >>> > > >> > the old config command.  Otherwise we have to explain when each
> >>> > > >> > tool should be used, which will be confusing to users.> >>
> >>> > > >> I'd like that, yes :).
> >>> > > >>
> >>> > > >> Cheers,
> >>> > > >> Viktor
> >>> > > >>
> >>> > > >> On Mon, May 7, 2018 at 6:52 PM, Colin McCabe <cmccabe@apache.org>
> >>> > > >> wrote:> >> > On Fri, May 4, 2018, at 05:49, Viktor Somogyi wrote:
> >>> > > >> >> Hi Colin,
> >>> > > >> >>
> >>> > > >> >> > Rather than breaking compatibility, we should simply add a new
> >>> > > >> >> > "incremental" boolean to AlterConfigsOptions.  Callers can set
> >>> > > >> >> > this boolean to true when they want the update to be
> >>> > > >> >> > incremental.  It should default to false so that old code
> >>> > > >> >> > continues to work.> >> >>
> >>> > > >> >> Agreed, let's do it this way.
> >>> > > >> >>
> >>> > > >> >> > Hmm.  I don't think AlterOperation is necessary.  If the user
> >>> > > >> >> > wants to delete a configuration key named "foo", they can
> >>> > > >> >> > create a ConfigEntry with name = "foo", value = null.> >> >>
> >>> > > >> >> AlterConfig's config type currently is string, so the only
> >>> > > >> >> possibility> >> >> is to use an empty string as changing the type to
> >>> > > >> >> nullable_string> >> >> could be breaking if the client code doesn't expect -1 as the
> >>> > > >> >> string> >> >> size. In the discussion thread earlier we had a conversation
> >>> > > >> >> about> >> >> this with Rajini, let me paste it here (so it gives some
> >>> > > >> >> context). At> >> >> that point I had the text "<default>" for this functionality:
> >>> > > >> >
> >>> > > >> > Hi Viktor,
> >>> > > >> >
> >>> > > >> > We are going to need to create a new version of
> >>> > > >> > AlterConfigsRequest to add the "incremental" boolean.  So while
> >>> > > >> > we're doing that, maybe we can change the type to
> >>> > > >> > NULLABLE_STRING.> >> >
> >>> > > >> >> "4. We use "<default>" internally to store default quotas and
> >>> > > >> >> other> >> >> defaults. But I don't think we should externalise that string.
> >>> > > >> >> We use empty> >> >> string elsewhere for indicating default, we can do the same
> >>> > > >> >> here.> >> >
> >>> > > >> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or
> >>> > > >> > "<default>" to indicate defaults, does it?> >> >
> >>> > > >> > There is a ConfigEntry class:
> >>> > > >> >
> >>> > > >> >  > @InterfaceStability.Evolving
> >>> > > >> >  > public class ConfigEntry {
> >>> > > >> >  >
> >>> > > >> >  >     private final String name;
> >>> > > >> >  >     private final String value;
> >>> > > >> >  >     private final ConfigSource source;
> >>> > > >> >  >     private final boolean isSensitive;
> >>> > > >> >  >     private final boolean isReadOnly;
> >>> > > >> >  >     private final List<ConfigSynonym> synonyms;
> >>> > > >> >
> >>> > > >> > and the ConfigSource enum indicates where the config came from:
> >>> > > >> >
> >>> > > >> >  >     /**
> >>> > > >> >  >      * Source of configuration entries.
> >>> > > >> >  >      */
> >>> > > >> >  >     public enum ConfigSource {
> >>> > > >> >  >         DYNAMIC_TOPIC_CONFIG,           // dynamic topic
> >>> > > >> >  >         config that is configured for a specific topic> >> >  >         DYNAMIC_BROKER_CONFIG,          // dynamic broker
> >>> > > >> >  >         config that is configured for a specific broker> >> >  >         DYNAMIC_DEFAULT_BROKER_CONFIG,  // dynamic broker
> >>> > > >> >  >         config that is configured as default for all brokers
> >>> > > >> >  >         in the cluster> >> >  >         STATIC_BROKER_CONFIG,           // static broker
> >>> > > >> >  >         config provided as broker properties at start up (e.g.
> >>> > > >> >  >         server.properties file)> >> >  >         DEFAULT_CONFIG,                 // built-in default
> >>> > > >> >  >         configuration for configs that have a default value> >> >  >         UNKNOWN                         // source unknown e.g.
> >>> > > >> >  >         in the ConfigEntry used for alter requests where
> >>> > > >> >  >         source is not set> >> >  >     }
> >>> > > >> >
> >>> > > >> > This comes from DescribeConfigsResponse.
> >>> > > >> > Unless I'm missing something, I think your suggestion to not
> >>> > > >> > expose "<default>" is already implemented?> >> >
> >>> > > >> >> And we use STRING rather than NULLABLE_STRING in describe
> >>> > > >> >> configs etc. So we> >> >> should probably do the same for quotas."
> >>> > > >> >
> >>> > > >> > I think nearly all responses treat ERROR_MESSAGE as a nullable
> >>> > > >> > string.  CommonFields#ERROR_MESSAGE, which is used by most of
> >>> > > >> > them, is a nullable string.  It's DescribeConfigsResponse that is
> >>> > > >> > the black sheep here.> >> >
> >>> > > >> >  >     public static final Field.NullableStr ERROR_MESSAGE = new
> >>> > > >> >  >     Field.NullableStr("error_message", "Response error
> >>> > > >> >  >     message");> >> >
> >>> > > >> >>
> >>> > > >> >> > Yeah, this might be an excessive maintenance burden.  Maybe we
> >>> > > >> >> > should get rid of the old zookeeper-based code, and just move
> >>> > > >> >> > towards having only a KIP-248-based tool.  It's a breaking
> >>> > > >> >> > change, but it's clear to users that it's occurring, and what
> >>> > > >> >> > the fix is (specifying --bootstrap-server instead of --
> >>> > > >> >> > zookeeper).> >> >>
> >>> > > >> >> Earlier Rajini raised a concern that direct zookeeper
> >>> > > >> >> interaction is> >> >> required to add the SCRAM credentials which will be used for
> >>> > > >> >> validation if inter-broker communication uses this auth method.
> >>> > > >> >> This> >> >> is currently done by the ConfigCommand. Therefore we can't
> >>> > > >> >> completely> >> >> get rid of it yet either.
> >>> > > >> >>
> >>> > > >> >> In my opinion though on a longer term (and this is now a bit
> >>> > > >> >> off-topic) Kafka shouldn't use Zookeeper as a credentials store,
> >>> > > >> >> just> >> >> provide an interface, so 3rd party authentication stores could
> >>> > > >> >> be> >> >> implemented. Then similarly to the authorizer we could have
> >>> > > >> >> Zookeeper> >> >> as a default though and a client that manages SCRAM credentials
> >>> > > >> >> in ZK.> >> >> From this perspective I'd leave the the command there but put a> >> >> warning that the tool is deprecated and should only be used for> >> >> setting up SCRAM credentials.
> >>> > > >> >> What do you think?
> >>> > > >> >
> >>> > > >> > What about writing a small script that just handles setting up
> >>> > > >> > SCRAM credentials?  It would probably be easier to maintain than
> >>> > > >> > the old config command.  Otherwise we have to explain when each
> >>> > > >> > tool should be used, which will be confusing to users.> >> >
> >>> > > >> > best,
> >>> > > >> > Colin
> >>> > > >> >
> >>> > > >> >>
> >>> > > >> >> Cheers,
> >>> > > >> >> Viktor
> >>> > > >> >>
> >>> > > >> >>
> >>> > > >> >> On Thu, May 3, 2018 at 7:47 PM, Colin McCabe
> >>> > > >> >> <cmccabe@apache.org> wrote:> >> >> > On Thu, May 3, 2018, at 05:11, Viktor Somogyi wrote:
> >>> > > >> >> >> @Magnus, yes that is correct. Thanks for your feedback.
> >>> > > >> >> >> Updated it with> >> >> >> this (which might be subject to change based on the
> >>> > > >> >> >> conversation with> >> >> >> Colin): "The changes done will be incremental in version 1,
> >>> > > >> >> >> opposed to the> >> >> >> atomic behavior in version 0. For instance in version 0
> >>> > > >> >> >> sending an update> >> >> >> for producer_byte_rate for userA would result in removing all
> >>> > > >> >> >> previous data> >> >> >> and setting userA's config with producer_byte_rate. Now in
> >>> > > >> >> >> version 1> >> >> >> opposed to version 0 it will add an extra config and keeps
> >>> > > >> >> >> other existing> >> >> >> configs."
> >>> > > >> >> >
> >>> > > >> >> > Hi Viktor,
> >>> > > >> >> >
> >>> > > >> >> > AdminClient#alterConfigs is a public API which users have
> >>> > > >> >> > already written code against.  If we silently change what it
> >>> > > >> >> > does to be incremental addition rather than complete
> >>> > > >> >> > replacement of the existing configuration, we will break all
> >>> > > >> >> > of that existing code.  If we do that, there is not even any
> >>> > > >> >> > way that users can write code to support both broker versions.
> >>> > > >> >> > AdminClient does not expose any API that users can use to
> >>> > > >> >> > check broker version.  I think that would be really bad for
> >>> > > >> >> > users.> >> >> >
> >>> > > >> >> > Rather than breaking compatibility, we should simply add a new
> >>> > > >> >> > "incremental" boolean to AlterConfigsOptions.  Callers can set
> >>> > > >> >> > this boolean to true when they want the update to be
> >>> > > >> >> > incremental.  It should default to false so that old code
> >>> > > >> >> > continues to work.> >> >> >
> >>> > > >> >> >>
> >>> > > >> >> >> @Colin,
> >>> > > >> >> >> yes, I have/had a hard time finding a place for this
> >>> > > >> >> >> operation. I think ADD> >> >> >> and DELETE should be on config level to allow complex use
> >>> > > >> >> >> cases (if someone> >> >> >> builds their own tool based on the AdminClient), so users can
> >>> > > >> >> >> add and> >> >> >> delete multiple configs in one request.
> >>> > > >> >> >
> >>> > > >> >> > Hmm.  I don't think AlterOperation is necessary.  If the user
> >>> > > >> >> > wants to delete a configuration key named "foo", they can
> >>> > > >> >> > create a ConfigEntry with name = "foo", value = null.> >> >> >
> >>> > > >> >> >> But also at the same time, SET is as you're suggesting really
> >>> > > >> >> >> seems like a> >> >> >> flag that tells the AdminClient/AdminManager how they should
> >>> > > >> >> >> behave.> >> >> >> However since the AdminClient matches protocol version with
> >>> > > >> >> >> the broker via> >> >> >> the API_VERSIONS request, I think it would be enough to
> >>> > > >> >> >> modify the> >> >> >> AdminManager that it should behave differently in case of an
> >>> > > >> >> >> increased> >> >> >> protocol versions, if there is this extra flag set through
> >>> > > >> >> >> AlterConfigOptions (AdminClient sets the flag on the
> >>> > > >> >> >> protocol, which will> >> >> >> be reflected after parsing in AdminManager). Also if we
> >>> > > >> >> >> target this change> >> >> >> to 2.0 (June?), then we might not need the extra flag but
> >>> > > >> >> >> make the behavior> >> >> >> break. What do you think?
> >>> > > >> >> >
> >>> > > >> >> > Right.  I think a flag in AlterConfigsRequest makes sense.
> >>> > > >> >> > AdminClient can set it based on a boolean field in
> >>> > > >> >> > AlterConfigsOptions.> >> >> >
> >>> > > >> >> >>
> >>> > > >> >> >> Keeping the --zookeeper option working is not infeasible of
> >>> > > >> >> >> course - and as> >> >> >> per the community's feedback it may be the better option.
> >>> > > >> >> >> Although one of> >> >> >> the goals is to put this new ConfigCommand to the tools
> >>> > > >> >> >> module, which> >> >> >> doesn't have the dependency on the server code, it would be a
> >>> > > >> >> >> bit harder.> >> >> >> Most likely I'd need to call into the Scala code with
> >>> > > >> >> >> reflection, which> >> >> >> could be quite complicated.
> >>> > > >> >> >
> >>> > > >> >> > Yeah, this might be an excessive maintenance burden.  Maybe we
> >>> > > >> >> > should get rid of the old zookeeper-based code, and just move
> >>> > > >> >> > towards having only a KIP-248-based tool.  It's a breaking
> >>> > > >> >> > change, but it's clear to users that it's occurring, and what
> >>> > > >> >> > the fix is (specifying --bootstrap-server instead of --
> >>> > > >> >> > zookeeper).> >> >> >
> >>> > > >> >> > best,
> >>> > > >> >> > Colin
> >>> > > >> >> >
> >>> > > >> >> >>
> >>> > > >> >> >> Also rewrote the request semantics, hopefully it's more clear
> >>> > > >> >> >> now.> >> >> >>
> >>> > > >> >> >> Let me know what do you think about this and thank you for
> >>> > > >> >> >> your feedback.> >> >> >>
> >>> > > >> >> >> Cheers,
> >>> > > >> >> >> Viktor
> >>> > > >> >> >>
> >>> > > >> >> >> On Thu, Apr 26, 2018 at 7:06 PM, Colin McCabe
> >>> > > >> >> >> <cmccabe@apache.org> wrote:> >> >> >>
> >>> > > >> >> >> > Hi Viktor,
> >>> > > >> >> >> >
> >>> > > >> >> >> > If I'm reading the KIP right, it looks like the new
> >>> > > >> >> >> > proposed verison of> >> >> >> > AlterConfigs sets an OperationType on a per-config basis:
> >>> > > >> >> >> >
> >>> > > >> >> >> >  > AlterConfigs Request (Version: 1) => [resources]
> >>> > > >> >> >> >  > validate_only> >> >> >> >  >   validate_only => BOOLEAN
> >>> > > >> >> >> >  >   resources => resource_type resource_name [configs]
> >>> > > >> >> >> >  >     resource_type => INT8
> >>> > > >> >> >> >  >     resource_name => STRING
> >>> > > >> >> >> >  >     configs => config_name config_value config_operation> >> >> >> >  >       config_name => STRING
> >>> > > >> >> >> >  >       config_value => STRING
> >>> > > >> >> >> >  >       config_operation => INT8 [NEW ADDITION]
> >>> > > >> >> >> >  >
> >>> > > >> >> >> >  > Request Semantics:
> >>> > > >> >> >> >  >
> >>> > > >> >> >> >  >      By default in the broker we parse an
> >>> > > >> >> >> >  >      AlterConfigRequest version 0> >> >> >> >  > with Unknown operation and handle it with the currently
> >>> > > >> >> >> >  > existing> >> >> >> > behavior.
> >>> > > >> >> >> >  > Version 1 requests however must have the operation set
> >>> > > >> >> >> >  > to other than> >> >> >> >  > Unknown, otherwise an InvalidRequestException will be
> >>> > > >> >> >> >  > thrown.> >> >> >> >  >          Set operation also does Add if needed to be
> >>> > > >> >> >> >  >          backward> >> >> >> > compatible
> >>> > > >> >> >> >  > with the existing ConfigCommand semantics.
> >>> > > >> >> >> >
> >>> > > >> >> >> > However, this seems like a configuration that should be
> >>> > > >> >> >> > global to the> >> >> >> > whole AlterConfigs request, right?  It doesn't make sense
> >>> > > >> >> >> > to have one> >> >> >> > configuration key use AlterOperation.Set and the other use> >> >> >> > AlterOperation.Add -- the Set one specifies that we should
> >>> > > >> >> >> > overwrite the> >> >> >> > whole node in ZK.
> >>> > > >> >> >> >
> >>> > > >> >> >> > Another consideration here is that we should do this in a
> >>> > > >> >> >> > compatible> >> >> >> > fashion in AdminClient.  Existing code that relies on the
> >>> > > >> >> >> > "set everything"> >> >> >> > behavior should not break.  The best way to do this is to
> >>> > > >> >> >> > add a boolean to> >> >> >> > ./clients/src/main/java/org/apache/kafka/clients/admin/Alt-
> >>> > > >> >> >> > erConfigsOptions.java> >> >> >> > , specifying whether we want to clear everything that
> >>> > > >> >> >> > hasn't been> >> >> >> > specified, or not.  This should default to true so that
> >>> > > >> >> >> > existing code can> >> >> >> > continue to work.... Unless we believe that the existing
> >>> > > >> >> >> > AlterConfigs> >> >> >> > behavior is so broken that it should be changed, even in a> >> >> >> > compatibility-breaking way.
> >>> > > >> >> >> >
> >>> > > >> >> >> > Similarly, for other tools, we managed to support both the
> >>> > > >> >> >> > zookeeper-based> >> >> >> > way and the new way in the same tool for a while.  This
> >>> > > >> >> >> > seems like> >> >> >> > something users would really want-- is it truly infeasible
> >>> > > >> >> >> > to do here?  The> >> >> >> > Java code could call into the Scala code as necessary when
> >>> > > >> >> >> > the zk flag was> >> >> >> > specified, right?
> >>> > > >> >> >> >
> >>> > > >> >> >> > best,
> >>> > > >> >> >> > Colin
> >>> > > >> >> >> >
> >>> > > >> >> >> >
> >>> > > >> >> >> > On Thu, Apr 26, 2018, at 01:47, Magnus Edenhill wrote:
> >>> > > >> >> >> > > Hi Viktor,
> >>> > > >> >> >> > >
> >>> > > >> >> >> > > after speaking to Rajini it seems like this KIP will
> >>> > > >> >> >> > > allow clients to> >> >> >> > > perform incremental configuration updates with
> >>> > > >> >> >> > > AlterConfigs, only> >> >> >> > providing
> >>> > > >> >> >> > > the settings
> >>> > > >> >> >> > > that it wants to change, as opposed to the current atomic
> >>> > > >> >> >> > > behaviour where> >> >> >> > > all settings
> >>> > > >> >> >> > > need to be provided to avoid having them revert to their
> >>> > > >> >> >> > > default values.> >> >> >> > >
> >>> > > >> >> >> > > If this is indeed the case, could you update the KIP to
> >>> > > >> >> >> > > make this more> >> >> >> > > clear?
> >>> > > >> >> >> > > I.e., that using Version 1 of AlterConfigs has the
> >>> > > >> >> >> > >     incremental behaviour,> >> >> >> > > while
> >>> > > >> >> >> > > version 0 is atomic.
> >>> > > >> >> >> > >
> >>> > > >> >> >> > > Thanks,
> >>> > > >> >> >> > > Magnus
> >>> > > >> >> >> > >
> >>> > > >> >> >> > >
> >>> > > >> >> >> > >
> >>> > > >> >> >> > >
> >>> > > >> >> >> > > 2018-04-16 13:27 GMT+02:00 Viktor Somogyi
> >>> > > >> >> >> > > <viktorsomogyi@gmail.com>:> >> >> >> > >
> >>> > > >> >> >> > > > Hi Rajini,
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > > The current ConfigCommand would still be possible to
> >>> > > >> >> >> > > > use, therefore> >> >> >> > those
> >>> > > >> >> >> > > > who wish to set up SCRAM or initial quotas would be
> >>> > > >> >> >> > > > able to continue> >> >> >> > doing
> >>> > > >> >> >> > > > it through kafka-run-class.sh.
> >>> > > >> >> >> > > > In an ideal world I'd keep it in the current
> >>> > > >> >> >> > > > ConfigCommand command so> >> >> >> > we
> >>> > > >> >> >> > > > wouldn't mix the zookeeper and admin client configs.
> >>> > > >> >> >> > > > Perhaps I could> >> >> >> > create
> >>> > > >> >> >> > > > a kafka-configs-zookeeper.sh shell script for
> >>> > > >> >> >> > > > shortening the> >> >> >> > > > kafka-run-class command.
> >>> > > >> >> >> > > > What do you and others think?
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > > Thanks,
> >>> > > >> >> >> > > > Viktor
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > > On Mon, Apr 16, 2018 at 10:15 AM, Rajini Sivaram <
> >>> > > >> >> >> > rajinisivaram@gmail.com>
> >>> > > >> >> >> > > > wrote:
> >>> > > >> >> >> > > >
> >>> > > >> >> >> > > > > Hi Viktor,
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > The KIP proposes to remove the ability to configure
> >>> > > >> >> >> > > > > using ZooKeeper.> >> >> >> > This
> >>> > > >> >> >> > > > > means we will no longer have the ability to start up
> >>> > > >> >> >> > > > > a cluster with> >> >> >> > SCRAM
> >>> > > >> >> >> > > > > credentials since we first need to create SCRAM
> >>> > > >> >> >> > > > > credentials before> >> >> >> > > > brokers
> >>> > > >> >> >> > > > > can start if the broker uses SCRAM for inter-broker
> >>> > > >> >> >> > > > > communication> >> >> >> > and we
> >>> > > >> >> >> > > > > need SCRAM credentials for the AdminClient before we
> >>> > > >> >> >> > > > > can create new> >> >> >> > ones.
> >>> > > >> >> >> > > > > For quotas as well, we will no longer be able to
> >>> > > >> >> >> > > > > configure quotas> >> >> >> > until
> >>> > > >> >> >> > > > at
> >>> > > >> >> >> > > > > least one broker has been started. Perhaps, we ought
> >>> > > >> >> >> > > > > to retain the> >> >> >> > > > ability
> >>> > > >> >> >> > > > > to configure using ZooKeeper for these initialization
> >>> > > >> >> >> > > > > scenarios and> >> >> >> > > > support
> >>> > > >> >> >> > > > > only AdminClient for dynamic updates?
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > What do others think?
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > Regards,
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > Rajini
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > On Sun, Apr 15, 2018 at 10:28 AM, Ted Yu
> >>> > > >> >> >> > > > > <yuzhihong@gmail.com>> >> >> >> > wrote:
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > > > > +1
> >>> > > >> >> >> > > > > > -------- Original message --------From: zhenya Sun
> >>> > > >> >> >> > > > > > <> >> >> >> > token01@126.com>
> >>> > > >> >> >> > > > > > Date: 4/15/18  12:42 AM  (GMT-08:00) To: dev
> >>> > > >> >> >> > > > > > <dev@kafka.apache.org> >> >> >> > >
> >>> > > >> >> >> > > > Cc:
> >>> > > >> >> >> > > > > > dev <dev@kafka.apache.org> Subject: Re: [VOTE] #2
> >>> > > >> >> >> > > > > > KIP-248: Create> >> >> >> > New
> >>> > > >> >> >> > > > > > ConfigCommand That Uses The New AdminClient
> >>> > > >> >> >> > > > > > non-binding +1
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > > from my iphone!
> >>> > > >> >> >> > > > > > On 04/15/2018 15:41, Attila Sasvári wrote:
> >>> > > >> >> >> > > > > > Thanks for updating the KIP.
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > > +1 (non-binding)
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > > Viktor Somogyi <viktorsomogyi@gmail.com> ezt írta
> >>> > > >> >> >> > > > > > (időpont: 2018.> >> >> >> > ápr.
> >>> > > >> >> >> > > > > 9.,
> >>> > > >> >> >> > > > > > H 16:49):
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > > > > Hi Magnus,
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > Thanks for the heads up, added the endianness to
> >>> > > >> >> >> > > > > > > the KIP. Here> >> >> >> > is the
> >>> > > >> >> >> > > > > > > current text:
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > "Double
> >>> > > >> >> >> > > > > > > A new type needs to be added to transfer quota
> >>> > > >> >> >> > > > > > > values. Since the> >> >> >> > > > > protocol
> >>> > > >> >> >> > > > > > > classes in Kafka already uses ByteBuffers it is
> >>> > > >> >> >> > > > > > > logical to use> >> >> >> > their
> >>> > > >> >> >> > > > > > > functionality for serializing doubles. The
> >>> > > >> >> >> > > > > > > serialization is> >> >> >> > > > basically a
> >>> > > >> >> >> > > > > > > representation of the specified floating-point
> >>> > > >> >> >> > > > > > > value according> >> >> >> > to the
> >>> > > >> >> >> > > > > > IEEE
> >>> > > >> >> >> > > > > > > 754 floating-point "double format" bit layout.
> >>> > > >> >> >> > > > > > > The ByteBuffer> >> >> >> > > > > serializer
> >>> > > >> >> >> > > > > > > writes eight bytes containing the given double
> >>> > > >> >> >> > > > > > > value, in Big> >> >> >> > Endian
> >>> > > >> >> >> > > > > byte
> >>> > > >> >> >> > > > > > > order, into this buffer at the current position,
> >>> > > >> >> >> > > > > > > and then> >> >> >> > increments
> >>> > > >> >> >> > > > > the
> >>> > > >> >> >> > > > > > > position by eight.
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > The implementation will be defined in
> >>> > > >> >> >> > > > > > > org.apache.kafka.common.protocol.types with the
> >>> > > >> >> >> > > > > > > other protocol> >> >> >> > types
> >>> > > >> >> >> > > > > > and it
> >>> > > >> >> >> > > > > > > will have no default value much like the other
> >>> > > >> >> >> > > > > > > types available> >> >> >> > in the
> >>> > > >> >> >> > > > > > > protocol."
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > Also, I haven't changed the protocol docs yet but
> >>> > > >> >> >> > > > > > > will do so in> >> >> >> > my PR
> >>> > > >> >> >> > > > > for
> >>> > > >> >> >> > > > > > > this feature.
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > Let me know if you'd still add something.
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > Regards,
> >>> > > >> >> >> > > > > > > Viktor
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > On Mon, Apr 9, 2018 at 3:32 PM, Magnus Edenhill <> >> >> >> > magnus@edenhill.se>
> >>> > > >> >> >> > > > > > > wrote:
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > > > > Hi Viktor,
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > > since serialization of floats isn't as straight
> >>> > > >> >> >> > > > > > > > forward as> >> >> >> > > > integers,
> >>> > > >> >> >> > > > > > > please
> >>> > > >> >> >> > > > > > > > specify the exact serialization format of
> >>> > > >> >> >> > > > > > > > DOUBLE in the> >> >> >> > protocol
> >>> > > >> >> >> > > > docs
> >>> > > >> >> >> > > > > > > > (e.g., IEEE 754),
> >>> > > >> >> >> > > > > > > > including endianness (big-endian please).
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > > This will help the non-java client ecosystem.
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > > Thanks,
> >>> > > >> >> >> > > > > > > > Magnus
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > > 2018-04-09 15:16 GMT+02:00 Viktor Somogyi <
> >>> > > >> >> >> > viktorsomogyi@gmail.com
> >>> > > >> >> >> > > > >:
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > > > > Hi Attila,
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > > > 1. It uses ByteBuffers, which in turn will
> >>> > > >> >> >> > > > > > > > >    use> >> >> >> > > > > > Double.doubleToLongBits
> >>> > > >> >> >> > > > > > > to
> >>> > > >> >> >> > > > > > > > > convert the double value to a long and that
> >>> > > >> >> >> > > > > > > > > long will be> >> >> >> > written
> >>> > > >> >> >> > > > in
> >>> > > >> >> >> > > > > > the
> >>> > > >> >> >> > > > > > > > > buffer. I'v updated the KIP with this.
> >>> > > >> >> >> > > > > > > > > 2. Good idea, modified it.
> >>> > > >> >> >> > > > > > > > > 3. During the discussion I remember we didn't
> >>> > > >> >> >> > > > > > > > >    really decide> >> >> >> > which
> >>> > > >> >> >> > > > > one
> >>> > > >> >> >> > > > > > > > would
> >>> > > >> >> >> > > > > > > > > be the better one but I agree that a wrapper
> >>> > > >> >> >> > > > > > > > > class that makes> >> >> >> > > > sure
> >>> > > >> >> >> > > > > > the
> >>> > > >> >> >> > > > > > > > list
> >>> > > >> >> >> > > > > > > > > that is used as a key is immutable is a good
> >>> > > >> >> >> > > > > > > > > idea and would> >> >> >> > ease
> >>> > > >> >> >> > > > > the
> >>> > > >> >> >> > > > > > > life
> >>> > > >> >> >> > > > > > > > > of people using the interface. Also more
> >>> > > >> >> >> > > > > > > > > importantly would> >> >> >> > make
> >>> > > >> >> >> > > > > sure
> >>> > > >> >> >> > > > > > > that
> >>> > > >> >> >> > > > > > > > > we always use the same hashCode. I have
> >>> > > >> >> >> > > > > > > > > created wrapper> >> >> >> > classes
> >>> > > >> >> >> > > > for
> >>> > > >> >> >> > > > > > the
> >>> > > >> >> >> > > > > > > > map
> >>> > > >> >> >> > > > > > > > > value as well but that was reverted to keep
> >>> > > >> >> >> > > > > > > > > things> >> >> >> > consistent.
> >>> > > >> >> >> > > > > > Although
> >>> > > >> >> >> > > > > > > > for
> >>> > > >> >> >> > > > > > > > > the key I think we wouldn't break
> >>> > > >> >> >> > > > > > > > > consistency. I updated the> >> >> >> > KIP.
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > > > Thanks,
> >>> > > >> >> >> > > > > > > > > Viktor
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > > > On Tue, Apr 3, 2018 at 1:27 PM, Attila
> >>> > > >> >> >> > > > > > > > > Sasvári <> >> >> >> > > > > asasvari@apache.org>
> >>> > > >> >> >> > > > > > > > > wrote:
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > Thanks for working on it Viktor.
> >>> > > >> >> >> > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > It looks good to me, but I have some
> >>> > > >> >> >> > > > > > > > > > questions:> >> >> >> > > > > > > > > > - I see a new type DOUBLE is used for
> >>> > > >> >> >> > > > > > > > > >   quota_value , and it> >> >> >> > is
> >>> > > >> >> >> > > > not
> >>> > > >> >> >> > > > > > > > listed
> >>> > > >> >> >> > > > > > > > > > among the primitive types on the Kafka
> >>> > > >> >> >> > > > > > > > > > protocol guide. Can> >> >> >> > you
> >>> > > >> >> >> > > > > add
> >>> > > >> >> >> > > > > > > some
> >>> > > >> >> >> > > > > > > > > > more details?
> >>> > > >> >> >> > > > > > > > > > - I am not sure that using an environment
> >>> > > >> >> >> > > > > > > > > >   (i.e.> >> >> >> > > > > > > > USE_OLD_COMMAND)variable
> >>> > > >> >> >> > > > > > > > > is
> >>> > > >> >> >> > > > > > > > > > the best way to control behaviour of kafka-
> >>> > > >> >> >> > > > > > > > > > config.sh . In> >> >> >> > other
> >>> > > >> >> >> > > > > > > scripts
> >>> > > >> >> >> > > > > > > > > > (e.g. console-consumer) an argument is
> >>> > > >> >> >> > > > > > > > > > passed (e.g.> >> >> >> > > > > > --new-consumer).
> >>> > > >> >> >> > > > > > > If
> >>> > > >> >> >> > > > > > > > > we
> >>> > > >> >> >> > > > > > > > > > still want to use it, then I would suggest
> >>> > > >> >> >> > > > > > > > > > something like> >> >> >> > > > > > > > > > USE_OLD_KAFKA_CONFIG_COMMAND. What do you
> >>> > > >> >> >> > > > > > > > > > think?> >> >> >> > > > > > > > > > - I have seen maps like
> >>> > > >> >> >> > > > > > > > > >   Map<List<ConfigResource>,> >> >> >> > > > > > > > Collection<QuotaType>>.
> >>> > > >> >> >> > > > > > > > > > If List<ConfigResource> is the key type,
> >>> > > >> >> >> > > > > > > > > > you should make> >> >> >> > sure
> >>> > > >> >> >> > > > > that
> >>> > > >> >> >> > > > > > > this
> >>> > > >> >> >> > > > > > > > > > List is immutable. Have you considered to
> >>> > > >> >> >> > > > > > > > > > introduce a new> >> >> >> > > > wrapper
> >>> > > >> >> >> > > > > > > > class?
> >>> > > >> >> >> > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > Regards,
> >>> > > >> >> >> > > > > > > > > > - Attila
> >>> > > >> >> >> > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > On Thu, Mar 29, 2018 at 1:46 PM, zhenya Sun
> >>> > > >> >> >> > > > > > > > > > <> >> >> >> > token01@126.com>
> >>> > > >> >> >> > > > > > wrote:
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > +1 (non-binding)
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > | |
> >>> > > >> >> >> > > > > > > > > > > zhenya Sun
> >>> > > >> >> >> > > > > > > > > > > 邮箱:token01@126.com
> >>> > > >> >> >> > > > > > > > > > > |
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > 签名由 网易邮箱大师 定制
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > On 03/29/2018 19:40, Sandor Murakozi
> >>> > > >> >> >> > > > > > > > > > > wrote:> >> >> >> > > > > > > > > > > +1 (non-binding)
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > Thanks for the KIP, Viktor
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > On Wed, Mar 21, 2018 at 5:41 PM, Viktor
> >>> > > >> >> >> > > > > > > > > > > Somogyi <> >> >> >> > > > > > > > > viktorsomogyi@gmail.com
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > wrote:
> >>> > > >> >> >> > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > > Hi Everyone,
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > > I've started a vote on KIP-248
> >>> > > >> >> >> > > > > > > > > > > > <https://cwiki.apache.org/conf
> >>> > > >> >> >> > luence/display/KAFKA/KIP-
> >>> > > >> >> >> > > > > > > > > > 248+-+Create+New+
> >>> > > >> >> >> > > > > > > > > > > > ConfigCommand+That+Uses+The+New+AdminC-
> >>> > > >> >> >> > > > > > > > > > > > lient#KIP-248-> >> >> >> > > > > > > > > > > > CreateNewConfigCommandThatUsesTheNewAd-
> >>> > > >> >> >> > > > > > > > > > > > minClient-> >> >> >> > > > > > DescribeQuotas>
> >>> > > >> >> >> > > > > > > > > > > > a few weeks ago but at the time I got a
> >>> > > >> >> >> > > > > > > > > > > > couple more> >> >> >> > > > comments
> >>> > > >> >> >> > > > > > and
> >>> > > >> >> >> > > > > > > it
> >>> > > >> >> >> > > > > > > > > was
> >>> > > >> >> >> > > > > > > > > > > > very close to 1.1 feature freeze,
> >>> > > >> >> >> > > > > > > > > > > > people were occupied> >> >> >> > with
> >>> > > >> >> >> > > > > > that,
> >>> > > >> >> >> > > > > > > > so
> >>> > > >> >> >> > > > > > > > > I
> >>> > > >> >> >> > > > > > > > > > > > wanted to restart the vote on this.
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > > *Summary of the KIP*
> >>> > > >> >> >> > > > > > > > > > > > For those who don't have context I
> >>> > > >> >> >> > > > > > > > > > > > thought I'd> >> >> >> > summarize it
> >>> > > >> >> >> > > > > in
> >>> > > >> >> >> > > > > > a
> >>> > > >> >> >> > > > > > > > few
> >>> > > >> >> >> > > > > > > > > > > > sentence.
> >>> > > >> >> >> > > > > > > > > > > > *Problem & Motivation: *The basic
> >>> > > >> >> >> > > > > > > > > > > > problem that the KIP> >> >> >> > > > tries
> >>> > > >> >> >> > > > > to
> >>> > > >> >> >> > > > > > > > solve
> >>> > > >> >> >> > > > > > > > > > is
> >>> > > >> >> >> > > > > > > > > > > > that kafka-configs.sh (which in turn
> >>> > > >> >> >> > > > > > > > > > > > uses the> >> >> >> > ConfigCommand
> >>> > > >> >> >> > > > > > > class)
> >>> > > >> >> >> > > > > > > > > uses
> >>> > > >> >> >> > > > > > > > > > a
> >>> > > >> >> >> > > > > > > > > > > > direct zookeeper connection. This is
> >>> > > >> >> >> > > > > > > > > > > > not desirable as> >> >> >> > > > getting
> >>> > > >> >> >> > > > > > > > around
> >>> > > >> >> >> > > > > > > > > > the
> >>> > > >> >> >> > > > > > > > > > > > broker opens up security issues and
> >>> > > >> >> >> > > > > > > > > > > > prevents the tool> >> >> >> > from
> >>> > > >> >> >> > > > > > being
> >>> > > >> >> >> > > > > > > > used
> >>> > > >> >> >> > > > > > > > > > in
> >>> > > >> >> >> > > > > > > > > > > > deployments where only the brokers are
> >>> > > >> >> >> > > > > > > > > > > > exposed to> >> >> >> > clients.
> >>> > > >> >> >> > > > > > Also a
> >>> > > >> >> >> > > > > > > > > > somewhat
> >>> > > >> >> >> > > > > > > > > > > > smaller motivation is to rewrite the
> >>> > > >> >> >> > > > > > > > > > > > tool in java as> >> >> >> > part
> >>> > > >> >> >> > > > of
> >>> > > >> >> >> > > > > > the
> >>> > > >> >> >> > > > > > > > > tools
> >>> > > >> >> >> > > > > > > > > > > > component so we can get rid of
> >>> > > >> >> >> > > > > > > > > > > > requiring the core> >> >> >> > module on
> >>> > > >> >> >> > > > > the
> >>> > > >> >> >> > > > > > > > > > classpath
> >>> > > >> >> >> > > > > > > > > > > > for the kafka-configs tool.
> >>> > > >> >> >> > > > > > > > > > > > *Solution:*
> >>> > > >> >> >> > > > > > > > > > > > - I've designed new 2 protocols:
> >>> > > >> >> >> > > > > > > > > > > >   DescribeQuotas and> >> >> >> > > > > > AlterQuotas.
> >>> > > >> >> >> > > > > > > > > > > > - Also redesigned the output format of
> >>> > > >> >> >> > > > > > > > > > > >   the command line> >> >> >> > > > tool
> >>> > > >> >> >> > > > > so
> >>> > > >> >> >> > > > > > > it
> >>> > > >> >> >> > > > > > > > > > provides
> >>> > > >> >> >> > > > > > > > > > > > a nicer result.
> >>> > > >> >> >> > > > > > > > > > > > - kafka-configs.[sh/bat] will use a new
> >>> > > >> >> >> > > > > > > > > > > >   java based> >> >> >> > > > > > ConfigCommand
> >>> > > >> >> >> > > > > > > > that
> >>> > > >> >> >> > > > > > > > > > is
> >>> > > >> >> >> > > > > > > > > > > > placed in tools.
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > > I'd be happy to receive any votes or
> >>> > > >> >> >> > > > > > > > > > > > feedback on this.> >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > > > > Regards,
> >>> > > >> >> >> > > > > > > > > > > > Viktor
> >>> > > >> >> >> > > > > > > > > > > >
> >>> > > >> >> >> > > > > > > > > >
> >>> > > >> >> >> > > > > > > > >
> >>> > > >> >> >> > > > > > > >
> >>> > > >> >> >> > > > > > >
> >>> > > >> >> >> > > > > >
> >>> > > >> >> >> > > > >
> >>> > > >> >> >> > > >
> >>> > > >> >> >> >
> >>> >
> 

Mime
View raw message