kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin McCabe <cmcc...@apache.org>
Subject Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Date Sat, 02 Dec 2017 22:15:23 GMT
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 <cmccabe@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.bentley@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 <rajinisivaram@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 <
> > > > rajinisivaram@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 <yuzhihong@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 <
> > > > > rajinisivaram@gmail.com
> > > > > >> >
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Ted,
> > > > > >> >
> > > > > >> > Have added an example for --delete-config.
> > > > > >> >
> > > > > >> > On Mon, Nov 20, 2017 at 7:42 PM, Ted Yu <yuzhihong@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
<
> > > > > >> > rajinisivaram@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 <
> > yuzhihong@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 <
> > > > > >> > > rajinisivaram@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
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> >

Mime
View raw message