Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi-Vass]

; > > >> >> Cheers,
> > > > >>> >>> > > >> >> Viktor
> > > > >>> >>> > > >> >>
> > > > >>> >>> > > >> >>
> > > > >>> >>> > > >> >> On Thu, May 3, 2018 at 7:47 PM, Colin McCabe
> > > > >>> >>> > > >> >>  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
> > > > >>> >>> > > &g

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Rajini Sivaram]

e 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
> > > >>> >>> > > >> >>  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
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Magnus Edenhill]

gt;>> >>> > > >> >> > 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.> 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Rajini Sivaram]

t; >>> >>> > > >> >> >
> >>> >>> > > >> >> > 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?
> >>> >>> > > >> >> >
> >>> >>> > > >> >> >

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

ay 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

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

sting 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.> >> >> >
&

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Attila Sasvári]

t;> >> >> 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
> >>> > > >> >> >> <cmcc...@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> >> >> >> >  &g

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

uot;, 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,
> >>> > > >> >> >> Vik

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

gt; > >> >> >> 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
>>> > > >> >> >> <cmcc...@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> >> >> >> &g

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

; >> >> >> 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
>> > > >> >> >> <cmcc...@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 => 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

; 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
>> > >> >> >> <cmcc...@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
>> > >> >> >> > compat

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

 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
> > > >> >> >> <cmcc...@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 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

; > 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
> > >> >> >> <cmcc...@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> >> 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

t; >> >> > 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
> >> >> >> <cmcc...@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> >> >> >> 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

ing 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/AlterConfigsOptions.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 
>> >> >> > > <viktorsomo...@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 
>> >

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

 --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 <cmcc...@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/AlterConfigsOptions.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

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

emantics.
>> >> >
>> >> > 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/AlterConfigsOptions.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 <viktorsomo...@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 <
>> >> > rajinisiva...@gmail.com>
>> >> > > > wrote:
>> >> > > >
>> >> > > > > Hi Viktor,
>> >> > > > >
>> >> > > > > The KIP proposes to remove the ability to configure using 
>> >> > > > > ZooKeeper.
>> &g

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

e 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/AlterConfigsOptions.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 <viktorsomo...@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
> >>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

;> > > 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 <viktorsomo...@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 <
>> > rajinisiva...@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 <yuzhih...@gmail.com>
>> > wrote:
>> > > > >
>> > > > > > +1
>> > > > > >  Original message From: zhenya Sun <
>> > toke...@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 <viktorsomo...@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:
>> > > > > > >
>> > > > > &

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

ded 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/AlterConfigsOptions.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 <viktorsomo...@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 <
> > rajinisiva...@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 <yuzhih...@gmail.com>
> > wrote:
> > > > >
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

our 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 <viktorsomo...@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 <
> rajinisiva...@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 <yuzhih...@gmail.com>
> wrote:
> > > >
> > > > > +1
> > > > >  Original message From: zhenya Sun <
> toke...@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 <viktorsomo...@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 defi

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

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/AlterConfigsOptions.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 <viktorsomo...@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 <rajinisiva...@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 <yuzhih...@gmail.com> wrote:
> > >
> > > > +1
> > > >  Original message Fr

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Magnus Edenhill]

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 <viktorsomo...@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 <rajinisiva...@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 <yuzhih...@gmail.com> wrote:
> >
> > > +1
> > > ---- Original message ----From: zhenya Sun <toke...@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 <viktorsomo...@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 <mag...@edenhill.se>
> > > > wrote:
> > > >
> > > > > Hi Viktor,
> > > > >
> > > > > since serialization of floats isn't as straight forward as
> integ

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

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 <rajinisiva...@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 <yuzhih...@gmail.com> wrote:
>
> > +1
> >  Original message From: zhenya Sun <toke...@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 <viktorsomo...@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 <mag...@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 <viktorsomo...@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 tha

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Rajini Sivaram]

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 <yuzhih...@gmail.com> wrote:

> +1
>  Original message From: zhenya Sun <toke...@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 <viktorsomo...@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 <mag...@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 <viktorsomo...@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 <asasv...@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
> > &

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Ted Yu]

+1
 Original message From: zhenya Sun <toke...@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 <viktorsomo...@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 <mag...@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 <viktorsomo...@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 <asasv...@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,
> > Collection>.
> > > > If List 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 <toke...@126.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > +1 （non-binding）
> > > > >
> > > > >
> > > > > | |
> > > > > zhenya Sun
&

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[zhenya Sun]

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  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 
> 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 :
> >
> > > 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 
> > > 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 > Collection>.
> > > > If List 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  wrote:
> > > > >
> > > > >
> > > > >
> > > > > +1 （non-binding）
> > > > >
> > > > >
> > > > > | |
> > > > > zhenya Sun
> > > > > 邮箱：toke...@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 <
> > > viktorsomo...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Everyone,
> > > > > >
> > > > > > I've started a vote on KIP-248
> > > > > >  > > > 248+-+Create+New+
> > > > > > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > > > > > CreateNewConfigCommandThatUsesTheNewAdminClient-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
> > 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Attila Sasvári]

Thanks for updating the KIP.

+1 (non-binding)

Viktor Somogyi  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 
> 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 :
> >
> > > 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 
> > > 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 > Collection>.
> > > > If List 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  wrote:
> > > > >
> > > > >
> > > > >
> > > > > +1 （non-binding）
> > > > >
> > > > >
> > > > > | |
> > > > > zhenya Sun
> > > > > 邮箱：toke...@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 <
> > > viktorsomo...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Everyone,
> > > > > >
> > > > > > I've started a vote on KIP-248
> > > > > >  > > > 248+-+Create+New+
> > > > > > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > > > > > CreateNewConfigCommandThatUsesTheNewAdminClient-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

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

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  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 :
>
> > 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 
> > 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 Collection>.
> > > If List 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  wrote:
> > > >
> > > >
> > > >
> > > > +1 （non-binding）
> > > >
> > > >
> > > > | |
> > > > zhenya Sun
> > > > 邮箱：toke...@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 <
> > viktorsomo...@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi Everyone,
> > > > >
> > > > > I've started a vote on KIP-248
> > > > >  > > 248+-+Create+New+
> > > > > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > > > > CreateNewConfigCommandThatUsesTheNewAdminClient-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 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Magnus Edenhill]

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 :

> 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 
> 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.
> > If List 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  wrote:
> > >
> > >
> > >
> > > +1 （non-binding）
> > >
> > >
> > > | |
> > > zhenya Sun
> > > 邮箱：toke...@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 <
> viktorsomo...@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi Everyone,
> > > >
> > > > I've started a vote on KIP-248
> > > >  > 248+-+Create+New+
> > > > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > > > CreateNewConfigCommandThatUsesTheNewAdminClient-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
> > > >
> >
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

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  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.
> If List 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  wrote:
> >
> >
> >
> > +1 （non-binding）
> >
> >
> > | |
> > zhenya Sun
> > 邮箱：toke...@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  >
> > wrote:
> >
> > > Hi Everyone,
> > >
> > > I've started a vote on KIP-248
> > >  248+-+Create+New+
> > > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > > CreateNewConfigCommandThatUsesTheNewAdminClient-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
> > >
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Attila Sasvári]

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.
If List 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  wrote:
>
>
>
> +1 （non-binding）
>
>
> | |
> zhenya Sun
> 邮箱：toke...@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 
> wrote:
>
> > Hi Everyone,
> >
> > I've started a vote on KIP-248
> >  > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > CreateNewConfigCommandThatUsesTheNewAdminClient-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
> >

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Rajini Sivaram]

+1 (binding)

Thanks for the KIP, Viktor!

On Tue, Apr 3, 2018 at 8:25 AM, Peter Toth  wrote:

> +1 (non binding)
>
> Thanks Viktor.
> Please remove "Unknown" AlterOperation type from Wire Format Types section
> as you did from the AdminClient APIs section.
>
>
> On Wed, Mar 21, 2018 at 5:41 PM, Viktor Somogyi 
> wrote:
>
> > Hi Everyone,
> >
> > I've started a vote on KIP-248
> >  > ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> > CreateNewConfigCommandThatUsesTheNewAdminClient-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
> >
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Peter Toth]

+1 (non binding)

Thanks Viktor.
Please remove "Unknown" AlterOperation type from Wire Format Types section
as you did from the AdminClient APIs section.


On Wed, Mar 21, 2018 at 5:41 PM, Viktor Somogyi 
wrote:

> Hi Everyone,
>
> I've started a vote on KIP-248
>  ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> CreateNewConfigCommandThatUsesTheNewAdminClient-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
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[zhenya Sun]

+1 （non-binding）


| |
zhenya Sun
邮箱：toke...@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 
wrote:

> Hi Everyone,
>
> I've started a vote on KIP-248
>  ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> CreateNewConfigCommandThatUsesTheNewAdminClient-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
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Sandor Murakozi]

+1 (non-binding)

Thanks for the KIP, Viktor

On Wed, Mar 21, 2018 at 5:41 PM, Viktor Somogyi 
wrote:

> Hi Everyone,
>
> I've started a vote on KIP-248
>  ConfigCommand+That+Uses+The+New+AdminClient#KIP-248-
> CreateNewConfigCommandThatUsesTheNewAdminClient-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
>