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