Since we are running out of time to get the whole ConfigCommand converted
to using the new AdminClient for 1.1.0 (KIP-248), we need a way to enable
ConfigCommand to handle broker config updates (implemented by KIP-226). As
a simple first step, it would make sense to use the existing ConfigCommand
tool to perform broker config updates enabled by this KIP. Since config
validation and password encryption are performed by the broker, this will
be easier to do with the new AdminClient. To do this, we need to add
command line options for new admin client to kafka-configs.sh. Dynamic
broker config updates alone will be done under KIP-226 using the new admin
client to make this feature usable.. The new command line options
(consistent with KIP-248) that will be added to ConfigCommand will be:

   - --bootstrap-server *host:port*
   - --adminclient.config *config-file*
   - --adminclient.properties *k1=v1,k2=v2*

If anyone has any concerns about these options being added to
kafka-configs.sh, please let me know. Otherwise, I will update KIP-226 and
add the options to one of the KIP-226 PRs.

Thank you,

Rajini

On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma <ism...@juma.me.uk> wrote:

> Thanks Rajini. Sounds good.
>
> Ismael
>
> On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram <rajinisiva...@gmail.com>
> wrote:
>
> > Hi Ismael,
> >
> > I have updated the KIP to use AES-256 if available and AES-128 otherwise
> > for password encryption. Looking at GCM, it looks like GCM is typically
> > used with a variable initialization vector, while we are using a random,
> > but constant IV per-password. Also, AES/GCM is not supported by Java7.
> > Since the authentication and performance benefits of GCM are not required
> > for this scenario, I am thinking I will leave the default as CBC, but
> make
> > sure we test GCM as well so that users have the choice.
> >
> > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe <cmcc...@apache.org>
> wrote:
> >
> > > Thanks, Rajini.  That makes sense.
> > >
> > > regards,
> > > Colin
> > >
> > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote:
> > > > Hi Colin,
> > > >
> > > > Thank you for reviewing.
> > > >
> > > > Yes, validation is done on the broker, not the client.
> > > >
> > > > All configs from ZooKeeper are processed and any config that could
> not
> > be
> > > > applied are logged as warnings. This includes any configs that are
> not
> > > > dynamic in the broker version or any configs that are not supported
> in
> > > the
> > > > broker version. If you downgrade to a version that is older than this
> > KIP
> > > > (1.0 for example), then you don't get any warnings however.
> > > >
> > > >
> > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe <cmcc...@apache.org>
> > wrote:
> > > >
> > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote:
> > > > > > Hi Rajini,
> > > > > >
> > > > > > Looking good. Just a few questions.
> > > > > >
> > > > > > 1. (Related to Jay's comment) Is the validate() method on
> > > Reconfigurable
> > > > > > necessary? I would have thought we'd validate using the
> ConfigDef.
> > > Do you
> > > > > > have a use case in mind in which the reconfigurable component
> only
> > > > > permits
> > > > > > certain reconfigurations?
> > > > >
> > > > > Hi,
> > > > >
> > > > > Sorry if this is a dumb question, but when we talk about validating
> > on
> > > the
> > > > > ConfigDef, we're talking about validating on the server side,
> right?
> > > The
> > > > > software on the client side might be older or newer than the
> software
> > > on
> > > > > the broker side, so it seems inadvisable to do the validation
> there.
> > > > >
> > > > > Also, after a software downgrade, when the broker is restarted, it
> > > might
> > > > > find that there is a configuration key that is stored in ZK that is
> > not
> > > > > dynamic in its (older) software version.  It seems like, with the
> > > current
> > > > > proposal, the broker will use the value found in the local
> > > configuration
> > > > > (config file) rather than the new ZK version.  Should the broker
> > print
> > > out
> > > > > a WARN message in that scenario?
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > > > 2. Should Reconfigurable extend Configurable or is the initial
> > > > > > configuration also done through reconfigure()? I ask because not
> > all
> > > > > > plugins interfaces currently extend Configurable (e.g.
> > > > > > KafkaPrincipalBuilder).
> > > > > > 3. You mentioned a couple changes to DescribeConfigsOptions and
> > > > > > DescribeConfigsResult. Perhaps we should list the changes
> > > explicitly? One
> > > > > > not totally obvious case is what the synonyms() getter would
> return
> > > if
> > > > > the
> > > > > > option is not specified (i.e. should it raise an exception or
> > return
> > > an
> > > > > > empty list?).
> > > > > > 4. Config entries in the DescribeConfigs response have an
> > is_default
> > > > > flag.
> > > > > > Could that be replaced with the more general config_source?
> > > > > > 5. Bit of an internal question, but how do you handle config
> > > > > dependencies?
> > > > > > For example, suppose I want to add a listener and configure its
> > > principal
> > > > > > builder at once. You'd have to validate the principal builder
> > config
> > > in
> > > > > the
> > > > > > context of the listener config, so I guess the order of the
> entries
> > > in
> > > > > > AlterConfigs is significant?
> > > > > > 6. KIP-48 (delegation tokens) gives us a master secret which is
> > > shared by
> > > > > > all brokers. Do you think we would make this dynamically
> > > configurable?
> > > > > > Alternatively, it might be possible to use it to encrypt the
> other
> > > > > > passwords we store in zookeeper.
> > > > > >
> > > > > > Thanks,
> > > > > > Jason
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram <
> > > > > rajinisiva...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Jay,
> > > > > > >
> > > > > > > Thank you for reviewing the KIP.
> > > > > > >
> > > > > > > 1) Yes, makes sense. I will update the PR. There are some
> config
> > > > > updates
> > > > > > > that may be allowed depending on the context (e.g. some
> security
> > > > > configs
> > > > > > > can be updated for new listeners, but not existing listeners).
> > > Perhaps
> > > > > it
> > > > > > > is ok to mark them dynamic in the documentation. AdminClient
> > would
> > > give
> > > > > > > appropriate error messages if the update is not allowed.
> > > > > > > 2) Internally, in the implementation, a mixture of direct
> config
> > > > > updates
> > > > > > > (e.g log config as you have pointed out) and reconfigure method
> > > > > invocations
> > > > > > > (e.g. SslFactory) are used. For configurable plugins (e.g.
> > metrics
> > > > > > > reporter), we require the Reconfigurable interface to ensure
> that
> > > we
> > > > > can
> > > > > > > validate any custom configs and avoid reconfiguration for
> plugin
> > > > > versions
> > > > > > > that don't support it.
> > > > > > >
> > > > > > >
> > > > > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps <j...@confluent.io>
> > > wrote:
> > > > > > >
> > > > > > > > Two thoughts on implementation (shouldn't effect the KIP):
> > > > > > > >
> > > > > > > >    1. It might be nice to add a parameter to ConfigDef which
> > says
> > > > > > > whether a
> > > > > > > >    configuration is dynamically updatable or not so that we
> can
> > > give
> > > > > > > error
> > > > > > > >    messages if it isn't and also have it reflected in the
> > > > > auto-generated
> > > > > > > > docs.
> > > > > > > >    2. For many systems they don't really need to take action
> > if a
> > > > > config
> > > > > > > >    changes, they just need to use the new value. Changing
> them
> > > all to
> > > > > > > >    Reconfigurable requires managing a fair amount of
> mutability
> > > in
> > > > > each
> > > > > > > > class
> > > > > > > >    that accepts changes. Some need this since they need to
> take
> > > > > actions
> > > > > > > > when a
> > > > > > > >    config changes, but it seems like many just need to update
> > > some
> > > > > value.
> > > > > > > > For
> > > > > > > >    the later you might just be able to do something like what
> > we
> > > do
> > > > > for
> > > > > > > >    LogConfig where there is a single CurrentConfig instance
> > that
> > > has
> > > > > a
> > > > > > > >    reference to the current KafkaConfig and always reference
> > your
> > > > > > > > configurable
> > > > > > > >    parameters via this (e.g. config.current.myConfig). Dunno
> if
> > > that
> > > > > is
> > > > > > > >    actually better, but thought I'd throw it out there.
> > > > > > > >
> > > > > > > > -Jay
> > > > > > > >
> > > > > > > > On Sun, Dec 10, 2017 at 8:09 AM, Rajini Sivaram <
> > > > > rajinisiva...@gmail.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Jun,
> > > > > > > > >
> > > > > > > > > Thank you!
> > > > > > > > >
> > > > > > > > > 5. Yes, that makes sense. Agree that we don't want to add
> > > protocol
> > > > > > > > changes
> > > > > > > > > to *UpdateMetadataRequest* in this KIP. I have moved the
> > > update of
> > > > > > > > > *log.message.format.version* and *inter.broker.protocol.
> > > version*
> > > > > to
> > > > > > > > reduce
> > > > > > > > > restarts during upgrade to* Future Work*. We can do this
> in a
> > > > > follow-on
> > > > > > > > > KIP.
> > > > > > > > >
> > > > > > > > > I will wait for another day to see if there are any more
> > > comments
> > > > > and
> > > > > > > > start
> > > > > > > > > vote on Tuesday if there are no other concerns.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Sat, Dec 9, 2017 at 12:22 AM, Jun Rao <j...@confluent.io
> >
> > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi, Rajini,
> > > > > > > > > >
> > > > > > > > > > Thanks for the reply. They all make sense.
> > > > > > > > > >
> > > > > > > > > > 5. Got it. Note that currently, only live brokers are
> > > registered
> > > > > in
> > > > > > > ZK.
> > > > > > > > > > Another thing is that I am not sure that we want every
> > > broker to
> > > > > read
> > > > > > > > all
> > > > > > > > > > broker registrations directly from ZK. It's probably
> better
> > > to
> > > > > have
> > > > > > > the
> > > > > > > > > > controller propagate this information. That will require
> > > > > changing the
> > > > > > > > > > UpdateMetadataRequest protocol though. So, I am not sure
> if
> > > you
> > > > > want
> > > > > > > to
> > > > > > > > > do
> > > > > > > > > > that in the same KIP.
> > > > > > > > > >
> > > > > > > > > > Jun
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram <
> > > > > > > > rajinisiva...@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Jun,
> > > > > > > > > > >
> > > > > > > > > > > Thank you for the review.
> > > > > > > > > > >
> > > > > > > > > > > 1. No, I am hoping to migrate partitions to new
> threads.
> > We
> > > > > just
> > > > > > > need
> > > > > > > > > to
> > > > > > > > > > > ensure they don't run concurrently.
> > > > > > > > > > >
> > > > > > > > > > > 2. AdminClient has a validateOnly option for
> > AlterConfigs.
> > > Any
> > > > > > > > > exceptions
> > > > > > > > > > > or return value of false from Reconfigurable#validate
> > would
> > > > > fail
> > > > > > > the
> > > > > > > > > > > AlterConfigsRequest.
> > > > > > > > > > >
> > > > > > > > > > > 3. Yes, we will support describe and alter of configs
> > with
> > > > > listener
> > > > > > > > > > prefix.
> > > > > > > > > > > We will not allow alterConfigs() of security configs
> > > without
> > > > > the
> > > > > > > > > listener
> > > > > > > > > > > prefix, since we need to prevent the whole cluster
> being
> > > made
> > > > > > > > unusable.
> > > > > > > > > > >
> > > > > > > > > > > 4. Thank you, will make a note of that.
> > > > > > > > > > >
> > > > > > > > > > > 5. When we are upgrading (from 1.0 to 2.0 for example),
> > my
> > > > > > > > > understanding
> > > > > > > > > > is
> > > > > > > > > > > that we set inter.broker.protocol.version=1.0, do a
> > > rolling
> > > > > > > upgrade
> > > > > > > > of
> > > > > > > > > > the
> > > > > > > > > > > whole cluster and when all brokers are at 2.0, we do
> > > another
> > > > > > > rolling
> > > > > > > > > > > upgrade with inter.broker.protocol.version=2.0.
> Jason's
> > > > > suggestion
> > > > > > > > was
> > > > > > > > > > to
> > > > > > > > > > > avoid the second rolling upgrade by enabling dynamic
> > > update of
> > > > > > > > > > > inter.broker.protocol.version. To set
> > > > > > > inter.broker.protocol.version=
> > > > > > > > > 2.0
> > > > > > > > > > > dynamically, we need to verify not just that the
> current
> > > > > broker is
> > > > > > > on
> > > > > > > > > > > version 2.0, but that all brokers int the cluster are
> on
> > > > > version
> > > > > > > 2.0
> > > > > > > > (I
> > > > > > > > > > > thought that was the reason for the second rolling
> > > upgrade).
> > > > > The
> > > > > > > > broker
> > > > > > > > > > > version in ZK would enable that verification before
> > > performing
> > > > > the
> > > > > > > > > > update.
> > > > > > > > > > >
> > > > > > > > > > > 6. The config source would be
> > STATIC_BROKER_CONFIG/DYNAMIC_
> > > > > > > > > > BROKER_CONFIG,
> > > > > > > > > > > the config name would be listener.name.listenerA.
> > configX.
> > > And
> > > > > > > > synonyms
> > > > > > > > > > > list
> > > > > > > > > > > in describeConfigs() would list
> listener.name.listenerA.
> > > > > configX
> > > > > > > as
> > > > > > > > > well
> > > > > > > > > > > as
> > > > > > > > > > > configX.
> > > > > > > > > > >
> > > > > > > > > > > 7. I think `default` is an overused terminology
> already.
> > > When
> > > > > > > configs
> > > > > > > > > are
> > > > > > > > > > > described, they return a flag indicating if the value
> is
> > a
> > > > > default.
> > > > > > > > And
> > > > > > > > > > in
> > > > > > > > > > > the broker, we have so many levels of configs already
> and
> > > we
> > > > > are
> > > > > > > > adding
> > > > > > > > > > so
> > > > > > > > > > > many more, that it may be better to use a different
> term.
> > > It
> > > > > > > doesn't
> > > > > > > > > have
> > > > > > > > > > > to be synonyms, but since we want to use the same term
> > for
> > > > > topics
> > > > > > > and
> > > > > > > > > > > brokers and we have listener configs which override
> > > > > non-prefixed
> > > > > > > > > security
> > > > > > > > > > > configs, perhaps it is ok?
> > > > > > > > > > >
> > > > > > > > > > > Regards,
> > > > > > > > > > >
> > > > > > > > > > > Rajini
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Dec 6, 2017 at 11:50 PM, Jun Rao <
> > j...@confluent.io
> > > >
> > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > A couple more things.
> > > > > > > > > > > >
> > > > > > > > > > > > 6. For the SSL/SASL configurations with the listener
> > > prefix,
> > > > > do
> > > > > > > we
> > > > > > > > > need
> > > > > > > > > > > > another level in config_source since it's neither
> topic
> > > nor
> > > > > > > broker?
> > > > > > > > > > > >
> > > > > > > > > > > > 7. For include_synonyms in DescribeConfigs, the name
> > > makes
> > > > > sense
> > > > > > > > for
> > > > > > > > > > the
> > > > > > > > > > > > topic level configs. Not sure if it makes sense for
> > other
> > > > > > > > > hierarchies.
> > > > > > > > > > > > Perhaps sth more generic like default will be better?
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > >
> > > > > > > > > > > > Jun
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Dec 6, 2017 at 3:41 PM, Jun Rao <
> > > j...@confluent.io>
> > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the kip. Looks good overall. A few
> > comments
> > > > > below.
> > > > > > > > > > > > >
> > > > > > > > > > > > > 1. "num.replica.fetchers: Affinity of partitions to
> > > threads
> > > > > > > will
> > > > > > > > be
> > > > > > > > > > > > > preserved for ordering." Does that mean the new
> > fetcher
> > > > > threads
> > > > > > > > > won't
> > > > > > > > > > > be
> > > > > > > > > > > > > used until new partitions are added? This may be
> > > limiting.
> > > > > > > > > > > > >
> > > > > > > > > > > > > 2. I am wondering how the result from
> > > > > > > > reporter.validate(Map<String,
> > > > > > > > > > ?>
> > > > > > > > > > > > > configs) will be used. If it returns false, do we
> > > return
> > > > > false
> > > > > > > to
> > > > > > > > > the
> > > > > > > > > > > > admin
> > > > > > > > > > > > > client for the validation call, which will seem a
> bit
> > > > > weird?
> > > > > > > > > > > > >
> > > > > > > > > > > > > 3. For the SSL and SASL configuration changes, do
> we
> > > > > support
> > > > > > > > those
> > > > > > > > > > with
> > > > > > > > > > > > > the listener prefix (e.g.,
> external-ssl-lisener.ssl.
> > > > > > > > > > > keystore.location).
> > > > > > > > > > > > > If so, do we plan to include them in the result of
> > > > > > > > > describeConfigs()?
> > > > > > > > > > > > >
> > > > > > > > > > > > > 4. "Updates to advertised.listeners will
> re-register
> > > the
> > > > > new
> > > > > > > > > listener
> > > > > > > > > > > in
> > > > > > > > > > > > > ZK". To support this, we will likely need
> additional
> > > logic
> > > > > in
> > > > > > > the
> > > > > > > > > > > > > controller such that the controller can broadcast
> the
> > > > > metadata
> > > > > > > > with
> > > > > > > > > > the
> > > > > > > > > > > > new
> > > > > > > > > > > > > listeners to every broker.
> > > > > > > > > > > > >
> > > > > > > > > > > > > 5. Including broker version in broker registration
> in
> > > ZK.
> > > > > I am
> > > > > > > > not
> > > > > > > > > > sure
> > > > > > > > > > > > > the usage of that. Each broker knows its version.
> So,
> > > is
> > > > > that
> > > > > > > for
> > > > > > > > > the
> > > > > > > > > > > > > controller?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Jun
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, Dec 5, 2017 at 11:05 AM, Colin McCabe <
> > > > > > > > cmcc...@apache.org>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > >> On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram
> wrote:
> > > > > > > > > > > > >> > Hi Colin,
> > > > > > > > > > > > >> >
> > > > > > > > > > > > >> > KAFKA-5722 already has an owner, so I didn't
> want
> > to
> > > > > confuse
> > > > > > > > the
> > > > > > > > > > two
> > > > > > > > > > > > >> > KIPs.  They can be done independently of each
> > > other. The
> > > > > > > goal
> > > > > > > > is
> > > > > > > > > > to
> > > > > > > > > > > > try
> > > > > > > > > > > > >> and
> > > > > > > > > > > > >> > validate every config to the minimum validation
> > > already
> > > > > in
> > > > > > > the
> > > > > > > > > > > broker
> > > > > > > > > > > > >> for
> > > > > > > > > > > > >> > the static configs, but in some cases to a more
> > > > > restrictive
> > > > > > > > > level.
> > > > > > > > > > > So
> > > > > > > > > > > > a
> > > > > > > > > > > > >> > typo like a file-not-found or class-not-found
> > would
> > > > > > > definitely
> > > > > > > > > > fail
> > > > > > > > > > > > the
> > > > > > > > > > > > >> > AlterConfigs request (validation is performed by
> > > > > > > AlterConfigs
> > > > > > > > > > > > regardless
> > > > > > > > > > > > >> > of validateOnly flag). I am working out the
> > > additional
> > > > > > > > > validation
> > > > > > > > > > I
> > > > > > > > > > > > can
> > > > > > > > > > > > >> > perform as I implement updates for each config.
> > For
> > > > > example,
> > > > > > > > > > > > >> > inter-broker keystore update will not be allowed
> > > unless
> > > > > it
> > > > > > > can
> > > > > > > > > be
> > > > > > > > > > > > >> > verified against the currently configured
> > > truststore.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> HI Rajini,
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I agree.  It's probably better to avoid expanding
> > the
> > > > > scope of
> > > > > > > > > > > KIP-226.
> > > > > > > > > > > > >> I hope we can get to KAFKA-5722 soon, though,
> since
> > it
> > > > > will
> > > > > > > > really
> > > > > > > > > > > > >> improve the user experience for this feature.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> regards,
> > > > > > > > > > > > >> Colin
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> >
> > > > > > > > > > > > >> > On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe <
> > > > > > > > > cmcc...@apache.org
> > > > > > > > > > >
> > > > > > > > > > > > >> wrote:
> > > > > > > > > > > > >> >
> > > > > > > > > > > > >> > > On Tue, Nov 28, 2017, at 14:48, Rajini Sivaram
> > > wrote:
> > > > > > > > > > > > >> > > > Hi Colin,
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > Thank you for reviewing the KIP.
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > *kaka-configs.sh* will be converted to use
> > > > > *AdminClient*
> > > > > > > > > under
> > > > > > > > > > > > >> > > > KAFKA-5722.
> > > > > > > > > > > > >> > > > This is targeted for the next release
> (1.1.0).
> > > Under
> > > > > > > this
> > > > > > > > > KIP,
> > > > > > > > > > > we
> > > > > > > > > > > > >> will
> > > > > > > > > > > > >> > > > implement *AdminClient#alterConfigs* for the
> > > dynamic
> > > > > > > > configs
> > > > > > > > > > > > listed
> > > > > > > > > > > > >> in
> > > > > > > > > > > > >> > > > the KIP. This will include validation of the
> > > > > configs and
> > > > > > > > > will
> > > > > > > > > > > > return
> > > > > > > > > > > > >> > > > appropriate errors if configs are invalid.
> > > > > Integration
> > > > > > > > tests
> > > > > > > > > > > will
> > > > > > > > > > > > >> also be
> > > > > > > > > > > > >> > > > added using AdmnClient. Only the actual
> > > conversion
> > > > > of
> > > > > > > > > > > > ConfigCommand
> > > > > > > > > > > > >> to
> > > > > > > > > > > > >> > > > use AdminClient will be left to be done
> under
> > > > > > > KAFKA-5722.
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > Hi Rajini,
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > It seems like there is no KIP yet for
> > KAFKA-5722.
> > > > > Does it
> > > > > > > > > make
> > > > > > > > > > > > sense
> > > > > > > > > > > > >> to
> > > > > > > > > > > > >> > > describe the conversion of kafka-configs.sh to
> > use
> > > > > > > > AdminClient
> > > > > > > > > > in
> > > > > > > > > > > > >> > > KIP-226?  Since the AlterConfigs RPCs already
> > > exist,
> > > > > it
> > > > > > > > should
> > > > > > > > > > be
> > > > > > > > > > > > >> pretty
> > > > > > > > > > > > >> > > straightforward.  This would also let us add
> > some
> > > > > > > > information
> > > > > > > > > > > about
> > > > > > > > > > > > >> how
> > > > > > > > > > > > >> > > errors will be handled, which is pretty
> > important
> > > for
> > > > > > > users.
> > > > > > > > > > For
> > > > > > > > > > > > >> > > example, will kafka-configs.sh give an error
> if
> > > the
> > > > > user
> > > > > > > > > makes a
> > > > > > > > > > > > typo
> > > > > > > > > > > > >> > > when setting a configuration?
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > Once KAFKA-5722 is implemented,*
> > > kafka-confgs.sh*
> > > > > can be
> > > > > > > > > used
> > > > > > > > > > to
> > > > > > > > > > > > >> obtain
> > > > > > > > > > > > >> > > > the current configuration, which can be
> > > redirected
> > > > > to a
> > > > > > > > text
> > > > > > > > > > > file
> > > > > > > > > > > > to
> > > > > > > > > > > > >> > > create a
> > > > > > > > > > > > >> > > > static *server.properties* file. This should
> > > help
> > > > > when
> > > > > > > > > > > downgrading
> > > > > > > > > > > > >> - but
> > > > > > > > > > > > >> > > > it does require brokers to be running. We
> can
> > > also
> > > > > > > > document
> > > > > > > > > > how
> > > > > > > > > > > to
> > > > > > > > > > > > >> obtain
> > > > > > > > > > > > >> > > > the properties using *zookeeper-shell.sh*
> > while
> > > > > > > > downgrading
> > > > > > > > > if
> > > > > > > > > > > > >> brokers
> > > > > > > > > > > > >> > > are
> > > > > > > > > > > > >> > > > down.
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > If we rename properties, we should add the
> new
> > > > > property
> > > > > > > to
> > > > > > > > > ZK
> > > > > > > > > > > > based
> > > > > > > > > > > > >> on
> > > > > > > > > > > > >> > > > the value of the old property when the
> > upgraded
> > > > > broker
> > > > > > > > > starts
> > > > > > > > > > > up.
> > > > > > > > > > > > >> But we
> > > > > > > > > > > > >> > > > would probably leave the old property as is.
> > > The old
> > > > > > > > > property
> > > > > > > > > > > will
> > > > > > > > > > > > >> be
> > > > > > > > > > > > >> > > returned
> > > > > > > > > > > > >> > > > and used as a synonym only as long as the
> > > broker is
> > > > > on a
> > > > > > > > > > version
> > > > > > > > > > > > >> where it
> > > > > > > > > > > > >> > > > is still valid. But it can remain in ZK and
> be
> > > > > updated
> > > > > > > if
> > > > > > > > > > > > >> downgrading -
> > > > > > > > > > > > >> > > > it will be up to the user to update the old
> > > > > property if
> > > > > > > > > > > > downgrading
> > > > > > > > > > > > >> or
> > > > > > > > > > > > >> > > > delete it if not needed. Renaming properties
> > is
> > > > > likely
> > > > > > > to
> > > > > > > > be
> > > > > > > > > > > > >> confusing
> > > > > > > > > > > > >> > > in any
> > > > > > > > > > > > >> > > > case even without dynamic configs, so
> > hopefully
> > > it
> > > > > will
> > > > > > > be
> > > > > > > > > > very
> > > > > > > > > > > > >> rare.
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > Sounds good.
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > best,
> > > > > > > > > > > > >> > > Colin
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > Rajini
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > On Tue, Nov 28, 2017 at 7:47 PM, Colin
> McCabe
> > <
> > > > > > > > > > > cmcc...@apache.org
> > > > > > > > > > > > >
> > > > > > > > > > > > >> > > wrote:
> > > > > > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > Hi Rajini,
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > This seems like a nice improvement!
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > One thing that is a bit concerning is
> that,
> > if
> > > > > > > > > > > > >> bin/kafka-configs.sh is
> > > > > > > > > > > > >> > > > > used, there is no  way for the broker to
> > give
> > > > > feedback
> > > > > > > > or
> > > > > > > > > > > error
> > > > > > > > > > > > >> > > > > messages.  The broker can't say "sorry, I
> > > can't
> > > > > > > > > reconfigure
> > > > > > > > > > > that
> > > > > > > > > > > > >> in
> > > > > > > > > > > > >> > > that
> > > > > > > > > > > > >> > > > > way."  Or even "that configuration
> property
> > > is not
> > > > > > > > > > > > reconfigurable
> > > > > > > > > > > > >> in
> > > > > > > > > > > > >> > > > > this version of the software."  It seems
> > like
> > > in
> > > > > the
> > > > > > > > > > examples
> > > > > > > > > > > > >> give,
> > > > > > > > > > > > >> > > > > users will simply set a configuration
> using
> > > > > > > > > > > > bin/kafka-configs.sh,
> > > > > > > > > > > > >> but
> > > > > > > > > > > > >> > > > > then they have to check the broker log
> files
> > > to
> > > > > see if
> > > > > > > > it
> > > > > > > > > > > could
> > > > > > > > > > > > >> > > actually
> > > > > > > > > > > > >> > > > > be applied.  And even if it couldn't be
> > > applied,
> > > > > then
> > > > > > > it
> > > > > > > > > > still
> > > > > > > > > > > > >> lingers
> > > > > > > > > > > > >> > > > > in ZooKeeper.
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > This seems like it would lead to a lot of
> > user
> > > > > > > > confusion,
> > > > > > > > > > > since
> > > > > > > > > > > > >> they
> > > > > > > > > > > > >> > > get
> > > > > > > > > > > > >> > > > > no feedback when reconfiguring something.
> > For
> > > > > > > example,
> > > > > > > > > > there
> > > > > > > > > > > > >> will be a
> > > > > > > > > > > > >> > > > > lot of scenarios where someone finds a
> > > > > reconfiguration
> > > > > > > > > > command
> > > > > > > > > > > > on
> > > > > > > > > > > > >> > > > > Google, runs it, but then it doesn't do
> > > anything
> > > > > > > because
> > > > > > > > > the
> > > > > > > > > > > > >> software
> > > > > > > > > > > > >> > > > > version is different.  And there's no
> error
> > > > > message or
> > > > > > > > > > > feedback
> > > > > > > > > > > > >> about
> > > > > > > > > > > > >> > > > > this.  It just doesn't work.
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > To prevent this, I think we should convert
> > > > > > > > > > > bin/kafka-configs.sh
> > > > > > > > > > > > >> to use
> > > > > > > > > > > > >> > > > > AdminClient's AlterConfigsRequest.  This
> > > allows
> > > > > us to
> > > > > > > > > detect
> > > > > > > > > > > > >> scenarios
> > > > > > > > > > > > >> > > > > where, because of a typo, different
> software
> > > > > version,
> > > > > > > > or a
> > > > > > > > > > > value
> > > > > > > > > > > > >> of the
> > > > > > > > > > > > >> > > > > wrong type (eg. string vs. int), the given
> > > > > > > configuration
> > > > > > > > > > > cannot
> > > > > > > > > > > > be
> > > > > > > > > > > > >> > > > > applied.  We really should convert
> > > > > kafka-configs.sh to
> > > > > > > > use
> > > > > > > > > > > > >> AdminClient
> > > > > > > > > > > > >> > > > > anyway, for all the usual reasons-- people
> > > want to
> > > > > > > lock
> > > > > > > > > down
> > > > > > > > > > > > >> ZooKeeper,
> > > > > > > > > > > > >> > > > > ACLs should be enforced, internal
> > > representations
> > > > > > > should
> > > > > > > > > be
> > > > > > > > > > > > >> hidden, we
> > > > > > > > > > > > >> > > > > should support environments where ZK is
> not
> > > > > exposed,
> > > > > > > > etc.
> > > > > > > > > > etc.
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > Another issue that I see here is, how does
> > > this
> > > > > > > interact
> > > > > > > > > > with
> > > > > > > > > > > > >> > > downgrade?
> > > > > > > > > > > > >> > > > >  Presumably, if the user downgrades to a
> > > version
> > > > > that
> > > > > > > > > > doesn't
> > > > > > > > > > > > >> support
> > > > > > > > > > > > >> > > > > KIP-226, all the dynamic configurations
> > > stored in
> > > > > ZK
> > > > > > > > > revert
> > > > > > > > > > to
> > > > > > > > > > > > >> whatever
> > > > > > > > > > > > >> > > > > value they have in the local config files.
> > > Do we
> > > > > need
> > > > > > > > to
> > > > > > > > > > > have a
> > > > > > > > > > > > >> > > utility
> > > > > > > > > > > > >> > > > > that can reify the actual applied
> > > configuration
> > > > > into a
> > > > > > > > > local
> > > > > > > > > > > > text
> > > > > > > > > > > > >> file,
> > > > > > > > > > > > >> > > > > to make downgrades less painful?
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > With regard to upgrades, what happens if
> we
> > > > > change the
> > > > > > > > > name
> > > > > > > > > > > of a
> > > > > > > > > > > > >> > > > > configuration key in the future?  For
> > > example, if
> > > > > we
> > > > > > > > > decide
> > > > > > > > > > to
> > > > > > > > > > > > >> rename
> > > > > > > > > > > > >> > > > > min.insync.replicas to
> min.in.sync.replicas,
> > > > > > > presumably
> > > > > > > > we
> > > > > > > > > > > will
> > > > > > > > > > > > >> > > > > deprecate the old key name.  And then
> > perhaps
> > > it
> > > > > will
> > > > > > > be
> > > > > > > > > > > removed
> > > > > > > > > > > > >> in a
> > > > > > > > > > > > >> > > > > future release, such as Apache Kafka 2.0.
> > In
> > > this
> > > > > > > > > scenario,
> > > > > > > > > > > > >> should the
> > > > > > > > > > > > >> > > > > Kafka upgrade process change the name of
> the
> > > > > > > > configuration
> > > > > > > > > > key
> > > > > > > > > > > > in
> > > > > > > > > > > > >> ZK
> > > > > > > > > > > > >> > > > > from min.insync.replicas to
> > > min.in.sync.replicas?
> > > > > > > > > Obviously
> > > > > > > > > > > > this
> > > > > > > > > > > > >> will
> > > > > > > > > > > > >> > > > > make downgrades harder, if so.  But if it
> > > doesn't,
> > > > > > > then
> > > > > > > > > > > removing
> > > > > > > > > > > > >> > > > > deprecated configuration key synonyms
> might
> > > become
> > > > > > > very
> > > > > > > > > > > > difficult.
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > best,
> > > > > > > > > > > > >> > > > > Colin
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > On Wed, Nov 22, 2017, at 12:52, Rajini
> > Sivaram
> > > > > wrote:
> > > > > > > > > > > > >> > > > > > Hi Tom,
> > > > > > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > No, I am not proposing this as a way to
> > > > > configure
> > > > > > > > > > > replication
> > > > > > > > > > > > >> quotas.
> > > > > > > > > > > > >> > > > > > When
> > > > > > > > > > > > >> > > > > > you describe broker configs using
> > > AdminClient,
> > > > > you
> > > > > > > > will
> > > > > > > > > > see
> > > > > > > > > > > > all
> > > > > > > > > > > > >> the
> > > > > > > > > > > > >> > > > > > configs
> > > > > > > > > > > > >> > > > > > persisted in /configs/brokers/<brokerId>
> > in
> > > > > > > ZooKeeper
> > > > > > > > > and
> > > > > > > > > > > this
> > > > > > > > > > > > >> > > includes
> > > > > > > > > > > > >> > > > > > leader.replication.throttled.rate,
> > > > > > > > > follower.replication.
> > > > > > > > > > > > >> > > throttled.rate
> > > > > > > > > > > > >> > > > > > etc. But
> > > > > > > > > > > > >> > > > > > the broker configs that can be altered
> > using
> > > > > > > > AdminClient
> > > > > > > > > > as
> > > > > > > > > > > a
> > > > > > > > > > > > >> result
> > > > > > > > > > > > >> > > of
> > > > > > > > > > > > >> > > > > > this KIP are those explicitly stated in
> > the
> > > KIP
> > > > > > > (does
> > > > > > > > > not
> > > > > > > > > > > > >> include
> > > > > > > > > > > > >> > > any of
> > > > > > > > > > > > >> > > > > > the quota configs).
> > > > > > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > Regards,
> > > > > > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > Rajini
> > > > > > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > On Wed, Nov 22, 2017 at 7:54 PM, Tom
> > > Bentley <
> > > > > > > > > > > > >> t.j.bent...@gmail.com>
> > > > > > > > > > > > >> > > > > > wrote:
> > > > > > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > Hi Rajini,
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > > > > Just to clarify, are you proposing
> this
> > > as a
> > > > > way
> > > > > > > to
> > > > > > > > > > > > configure
> > > > > > > > > > > > >> > > > > interbroker
> > > > > > > > > > > > >> > > > > > > throttling/quotas? I don't think you
> > are,
> > > just
> > > > > > > > wanted
> > > > > > > > > to
> > > > > > > > > > > > check
> > > > > > > > > > > > >> > > (since
> > > > > > > > > > > > >> > > > > > > KIP-179 proposes a different mechanism
> > for
> > > > > setting
> > > > > > > > > them
> > > > > > > > > > > > which
> > > > > > > > > > > > >> > > supports
> > > > > > > > > > > > >> > > > > > > their automatic removal).
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > > > > Cheers,
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > > > > Tom
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > > > > On 22 November 2017 at 18:28, Rajini
> > > Sivaram <
> > > > > > > > > > > > >> > > rajinisiva...@gmail.com>
> > > > > > > > > > > > >> > > > > > > wrote:
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > > > > > I have made an update to the KIP to
> > > > > optionally
> > > > > > > > > return
> > > > > > > > > > > all
> > > > > > > > > > > > >> the
> > > > > > > > > > > > >> > > config
> > > > > > > > > > > > >> > > > > > > > synonyms in
> *DescribeConfigsResponse*.
> > > The
> > > > > > > > synonyms
> > > > > > > > > > are
> > > > > > > > > > > > >> returned
> > > > > > > > > > > > >> > > in
> > > > > > > > > > > > >> > > > > the
> > > > > > > > > > > > >> > > > > > > > order of precedence.
> > > AlterConfigsResponse
> > > > > will
> > > > > > > not
> > > > > > > > > be
> > > > > > > > > > > > >> modified
> > > > > > > > > > > > >> > > by the
> > > > > > > > > > > > >> > > > > > > KIP.
> > > > > > > > > > > > >> > > > > > > > Since many configs already have
> > various
> > > > > > > overrides
> > > > > > > > > > (e.g.
> > > > > > > > > > > > >> topic
> > > > > > > > > > > > >> > > configs
> > > > > > > > > > > > >> > > > > > > with
> > > > > > > > > > > > >> > > > > > > > broker overrides, security
> properties
> > > with
> > > > > > > > listener
> > > > > > > > > > name
> > > > > > > > > > > > >> > > overrides)
> > > > > > > > > > > > >> > > > > and
> > > > > > > > > > > > >> > > > > > > we
> > > > > > > > > > > > >> > > > > > > > will be adding more levels with
> > dynamic
> > > > > configs,
> > > > > > > > it
> > > > > > > > > > will
> > > > > > > > > > > > be
> > > > > > > > > > > > >> > > useful to
> > > > > > > > > > > > >> > > > > > > > obtain the full list in order of
> > > precedence.
> > > > > > > > > > > > >> > > > > > > >
> > > > > > > > > > > > >> > > > > > > > On Tue, Nov 21, 2017 at 11:24 AM,
> > Rajini
> > > > > > > Sivaram <
> > > > > > > > > > > > >> > > > > > > rajinisiva...@gmail.com>
> > > > > > > > > > > > >> > > > > > > > wrote:
> > > > > > > > > > > > >> > > > > > > >
> > > > > > > > > > > > >> > > > > > > > > Hi Ted,
> > > > > > > > > > > > >> > > > > > > > >
> > > > > > > > > > > > >> > > > > > > > > You can quote the config name, but
> > it
> > > is
> > > > > not
> > > > > > > > > > necessary
> > > > > > > > > > > > for
> > > > > > > > > > > > >> > > > > deleting a
> > > > > > > > > > > > >> > > > > > > > > config since the name doesn't
> > contain
> > > any
> > > > > > > > special
> > > > > > > > > > > > >> characters
> > > > > > > > > > > > >> > > that
> > > > > > > > > > > > >> > > > > > > > requires
> > > > > > > > > > > > >> > > > > > > > > quoting.
> > > > > > > > > > > > >> > > > > > > > >
> > > > > > > > > > > > >> > > > > > > > > On Mon, Nov 20, 2017 at 9:20 PM,
> Ted
> > > Yu <
> > > > > > > > > > > > >> yuzhih...@gmail.com>
> > > > > > > > > > > > >> > > > > wrote:
> > > > > > > > > > > > >> > > > > > > > >
> > > > > > > > > > > > >> > > > > > > > >> Thanks for the quick response.
> > > > > > > > > > > > >> > > > > > > > >>
> > > > > > > > > > > > >> > > > > > > > >> It seems the config following
> > > > > --delete-config
> > > > > > > > > > should
> > > > > > > > > > > be
> > > > > > > > > > > > >> > > quoted.
> > > > > > > > > > > > >> > > > > > > > >>
> > > > > > > > > > > > >> > > > > > > > >> Cheers
> > > > > > > > > > > > >> > > > > > > > >>
> > > > > > > > > > > > >> > > > > > > > >> On Mon, Nov 20, 2017 at 12:02 PM,
> > > Rajini
> > > > > > > > Sivaram
> > > > > > > > > <
> > > > > > > > > > > > >> > > > > > > > rajinisiva...@gmail.com
> > > > > > > > > > > > >> > > > > > > > >> >
> > > > > > > > > > > > >> > > > > > > > >> wrote:
> > > > > > > > > > > > >> > > > > > > > >>
> > > > > > > > > > > > >> > > > > > > > >> > Ted,
> > > > > > > > > > > > >> > > > > > > > >> >
> > > > > > > > > > > > >> > > > > > > > >> > Have added an example for
> > > > > --delete-config.
> > > > > > > > > > > > >> > > > > > > > >> >
> > > > > > > > > > > > >> > > > > > > > >> > On Mon, Nov 20, 2017 at 7:42
> PM,
> > > Ted
> > > > > Yu <
> > > > > > > > > > > > >> > > yuzhih...@gmail.com>
> > > > > > > > > > > > >> > > > > > > wrote:
> > > > > > > > > > > > >> > > > > > > > >> >
> > > > > > > > > > > > >> > > > > > > > >> > > bq. There is a
> --delete-config
> > > option
> > > > > > > > > > > > >> > > > > > > > >> > >
> > > > > > > > > > > > >> > > > > > > > >> > > Consider adding a sample with
> > the
> > > > > above
> > > > > > > > > option
> > > > > > > > > > to
> > > > > > > > > > > > >> the KIP.
> > > > > > > > > > > > >> > > > > > > > >> > >
> > > > > > > > > > > > >> > > > > > > > >> > > Thanks
> > > > > > > > > > > > >> > > > > > > > >> > >
> > > > > > > > > > > > >> > > > > > > > >> > > On Mon, Nov 20, 2017 at 11:36
> > AM,
> > > > > Rajini
> > > > > > > > > > Sivaram
> > > > > > > > > > > <
> > > > > > > > > > > > >> > > > > > > > >> > rajinisiva...@gmail.com>
> > > > > > > > > > > > >> > > > > > > > >> > > wrote:
> > > > > > > > > > > > >> > > > > > > > >> > >
> > > > > > > > > > > > >> > > > > > > > >> > > > Hi Ted,
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > Thank you for reviewing the
> > > KIP.
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > *Would decreasing
> network/IO
> > > > > threads be
> > > > > > > > > > > supported
> > > > > > > > > > > > >> ?*
> > > > > > > > > > > > >> > > > > > > > >> > > > Yes, As described in the
> KIP,
> > > some
> > > > > > > > > > connections
> > > > > > > > > > > > >> will be
> > > > > > > > > > > > >> > > > > closed if
> > > > > > > > > > > > >> > > > > > > > >> > network
> > > > > > > > > > > > >> > > > > > > > >> > > > thread count is reduced
> (and
> > > > > > > > reconnections
> > > > > > > > > > will
> > > > > > > > > > > > be
> > > > > > > > > > > > >> > > > > processed on
> > > > > > > > > > > > >> > > > > > > > >> > remaining
> > > > > > > > > > > > >> > > > > > > > >> > > > threads)
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > *What if some keys in
> configs
> > > are
> > > > > not
> > > > > > > in
> > > > > > > > > the
> > > > > > > > > > > Set
> > > > > > > > > > > > >> > > returned
> > > > > > > > > > > > >> > > > > > > > >> > > > by reconfigurableConfigs()?
> > > Would
> > > > > > > > exception
> > > > > > > > > > be
> > > > > > > > > > > > >> thrown ?*
> > > > > > > > > > > > >> > > > > > > > >> > > > No,
> *reconfigurableConfigs()
> > > *will
> > > > > be
> > > > > > > > used
> > > > > > > > > to
> > > > > > > > > > > > >> decide
> > > > > > > > > > > > >> > > which
> > > > > > > > > > > > >> > > > > > > classes
> > > > > > > > > > > > >> > > > > > > > >> are
> > > > > > > > > > > > >> > > > > > > > >> > > > notified when a
> configuration
> > > > > update is
> > > > > > > > > > made*.
> > > > > > > > > > > > >> > > > > > > > >> > **reconfigure(Map<String,
> > > > > > > > > > > > >> > > > > > > > >> > > ?>
> > > > > > > > > > > > >> > > > > > > > >> > > > configs)* will be invoked
> > with
> > > all
> > > > > of
> > > > > > > the
> > > > > > > > > > > > >> configured
> > > > > > > > > > > > >> > > > > configs of
> > > > > > > > > > > > >> > > > > > > > the
> > > > > > > > > > > > >> > > > > > > > >> > > broker,
> > > > > > > > > > > > >> > > > > > > > >> > > >  similar to
> > > > > *configure(Map<String, ?>
> > > > > > > > > > > configs).
> > > > > > > > > > > > >> *For
> > > > > > > > > > > > >> > > > > example,
> > > > > > > > > > > > >> > > > > > > > when
> > > > > > > > > > > > >> > > > > > > > >> > > > *SslChannelBuilder* is made
> > > > > > > > reconfigurable,
> > > > > > > > > > it
> > > > > > > > > > > > >> could
> > > > > > > > > > > > >> > > just
> > > > > > > > > > > > >> > > > > > > create a
> > > > > > > > > > > > >> > > > > > > > >> new
> > > > > > > > > > > > >> > > > > > > > >> > > > SslFactory with the latest
> > > configs,
> > > > > > > using
> > > > > > > > > the
> > > > > > > > > > > > same
> > > > > > > > > > > > >> code
> > > > > > > > > > > > >> > > as
> > > > > > > > > > > > >> > > > > > > > >> > *configure()*.
> > > > > > > > > > > > >> > > > > > > > >> > > > We avoid reconfiguring
> > > > > > > *SslChannelBuilder
> > > > > > > > > > > > >> > > *unnecessarily*,
> > > > > > > > > > > > >> > > > > *for
> > > > > > > > > > > > >> > > > > > > > >> example
> > > > > > > > > > > > >> > > > > > > > >> > > if
> > > > > > > > > > > > >> > > > > > > > >> > > > a topic config has changed,
> > > since
> > > > > topic
> > > > > > > > > > configs
> > > > > > > > > > > > >> are not
> > > > > > > > > > > > >> > > > > listed
> > > > > > > > > > > > >> > > > > > > in
> > > > > > > > > > > > >> > > > > > > > >> the
> > > > > > > > > > > > >> > > > > > > > >> > > > *SslChannelBuilder#**
> > > > > > > > > > reconfigurableConfigs().*
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > *The sample commands for
> > > > > > > > bin/kafka-configs
> > > > > > > > > > > > include
> > > > > > > > > > > > >> > > > > > > '--add-config'.
> > > > > > > > > > > > >> > > > > > > > >> > Would
> > > > > > > > > > > > >> > > > > > > > >> > > > there be '--remove-config'
> ?*
> > > > > > > > > > > > >> > > > > > > > >> > > > bin/kafka-configs.sh is an
> > > existing
> > > > > > > tool
> > > > > > > > > > whose
> > > > > > > > > > > > >> > > parameters
> > > > > > > > > > > > >> > > > > will
> > > > > > > > > > > > >> > > > > > > not
> > > > > > > > > > > > >> > > > > > > > >> be
> > > > > > > > > > > > >> > > > > > > > >> > > > modified by this KIP. There
> > is
> > > a
> > > > > > > > > > > --delete-config
> > > > > > > > > > > > >> option.
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > *ssl.keystore.password
> > appears
> > > a
> > > > > few
> > > > > > > > lines
> > > > > > > > > > > above.
> > > > > > > > > > > > >> Would
> > > > > > > > > > > > >> > > > > there be
> > > > > > > > > > > > >> > > > > > > > any
> > > > > > > > > > > > >> > > > > > > > >> > > > issue with mixture of
> > > connections
> > > > > (with
> > > > > > > > old
> > > > > > > > > > and
> > > > > > > > > > > > new
> > > > > > > > > > > > >> > > > > password) ?*
> > > > > > > > > > > > >> > > > > > > > >> > > > No, passwords (and the
> actual
> > > > > keystore)
> > > > > > > > are
> > > > > > > > > > > only
> > > > > > > > > > > > >> used
> > > > > > > > > > > > >> > > during
> > > > > > > > > > > > >> > > > > > > > >> > > > authentication. Any channel
> > > created
> > > > > > > using
> > > > > > > > > the
> > > > > > > > > > > old
> > > > > > > > > > > > >> > > SslFactory
> > > > > > > > > > > > >> > > > > > > will
> > > > > > > > > > > > >> > > > > > > > >> not
> > > > > > > > > > > > >> > > > > > > > >> > be
> > > > > > > > > > > > >> > > > > > > > >> > > > impacted.
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > Regards,
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > Rajini
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > On Mon, Nov 20, 2017 at
> 4:39
> > > PM,
> > > > > Ted
> > > > > > > Yu <
> > > > > > > > > > > > >> > > > > yuzhih...@gmail.com>
> > > > > > > > > > > > >> > > > > > > > >> wrote:
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > bq. (e.g. increase
> > network/IO
> > > > > > > threads)
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > Would decreasing
> network/IO
> > > > > threads
> > > > > > > be
> > > > > > > > > > > > supported
> > > > > > > > > > > > >> ?
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > bq.     void
> > > > > reconfigure(Map<String,
> > > > > > > ?>
> > > > > > > > > > > > configs);
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > What if some keys in
> > configs
> > > are
> > > > > not
> > > > > > > in
> > > > > > > > > the
> > > > > > > > > > > Set
> > > > > > > > > > > > >> > > returned
> > > > > > > > > > > > >> > > > > by
> > > > > > > > > > > > >> > > > > > > > >> > > > > reconfigurableConfigs()
> > > > > > > > > > > > >> > > > > > > > >> > > > > ? Would exception be
> > thrown ?
> > > > > > > > > > > > >> > > > > > > > >> > > > > If so, please specify
> which
> > > > > exception
> > > > > > > > > would
> > > > > > > > > > > be
> > > > > > > > > > > > >> thrown.
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > The sample commands for
> > > > > > > > bin/kafka-configs
> > > > > > > > > > > > include
> > > > > > > > > > > > >> > > > > > > > '--add-config'.
> > > > > > > > > > > > >> > > > > > > > >> > > > > Would there be
> > > '--remove-config'
> > > > > ?
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > bq. Existing connections
> > will
> > > > > not be
> > > > > > > > > > > affected,
> > > > > > > > > > > > >> new
> > > > > > > > > > > > >> > > > > connections
> > > > > > > > > > > > >> > > > > > > > >> will
> > > > > > > > > > > > >> > > > > > > > >> > use
> > > > > > > > > > > > >> > > > > > > > >> > > > the
> > > > > > > > > > > > >> > > > > > > > >> > > > > new keystore.
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > ssl.keystore.password
> > > appears a
> > > > > few
> > > > > > > > lines
> > > > > > > > > > > > above.
> > > > > > > > > > > > >> Would
> > > > > > > > > > > > >> > > > > there
> > > > > > > > > > > > >> > > > > > > be
> > > > > > > > > > > > >> > > > > > > > >> any
> > > > > > > > > > > > >> > > > > > > > >> > > issue
> > > > > > > > > > > > >> > > > > > > > >> > > > > with mixture of
> connections
> > > > > (with old
> > > > > > > > and
> > > > > > > > > > new
> > > > > > > > > > > > >> > > password) ?
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > Cheers
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > On Mon, Nov 20, 2017 at
> > 5:57
> > > AM,
> > > > > > > Rajini
> > > > > > > > > > > > Sivaram <
> > > > > > > > > > > > >> > > > > > > > >> > > rajinisiva...@gmail.com
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > wrote:
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > Hi all,
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > I have submitted
> KIP-226
> > to
> > > > > enable
> > > > > > > > > > dynamic
> > > > > > > > > > > > >> > > > > reconfiguration
> > > > > > > > > > > > >> > > > > > > of
> > > > > > > > > > > > >> > > > > > > > >> > brokers
> > > > > > > > > > > > >> > > > > > > > >> > > > > > without restart:
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > https://cwiki.apache.org/
> > > > > > > > > > > > >> > > confluence/display/KAFKA/KIP-
> > > > > > > > > > > > >> > > > > > > > >> > > > > > 226+-+Dynamic+Broker+
> > > > > Configuration
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > The KIP proposes to
> > extend
> > > the
> > > > > > > > current
> > > > > > > > > > > > dynamic
> > > > > > > > > > > > >> > > > > replication
> > > > > > > > > > > > >> > > > > > > > quota
> > > > > > > > > > > > >> > > > > > > > >> > > > > > configuration for
> brokers
> > > to
> > > > > > > support
> > > > > > > > > > > dynamic
> > > > > > > > > > > > >> > > > > reconfiguration
> > > > > > > > > > > > >> > > > > > > > of
> > > > > > > > > > > > >> > > > > > > > >> a
> > > > > > > > > > > > >> > > > > > > > >> > > > limited
> > > > > > > > > > > > >> > > > > > > > >> > > > > > set of configuration
> > > options
> > > > > that
> > > > > > > are
> > > > > > > > > > > > typically
> > > > > > > > > > > > >> > > updated
> > > > > > > > > > > > >> > > > > > > during
> > > > > > > > > > > > >> > > > > > > > >> the
> > > > > > > > > > > > >> > > > > > > > >> > > > > lifetime
> > > > > > > > > > > > >> > > > > > > > >> > > > > > of a broker.
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > Feedback and
> suggestions
> > > are
> > > > > > > welcome.
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > Thank you...
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > Regards,
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > > > Rajini
> > > > > > > > > > > > >> > > > > > > > >> > > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > > >
> > > > > > > > > > > > >> > > > > > > > >> > > >
> > > > > > > > > > > > >> > > > > > > > >> > >
> > > > > > > > > > > > >> > > > > > > > >> >
> > > > > > > > > > > > >> > > > > > > > >>
> > > > > > > > > > > > >> > > > > > > > >
> > > > > > > > > > > > >> > > > > > > > >
> > > > > > > > > > > > >> > > > > > > >
> > > > > > > > > > > > >> > > > > > >
> > > > > > > > > > > > >> > > > >
> > > > > > > > > > > > >> > >
> > > > > > > > > > > > >>
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > >
> >
>

Reply via email to