kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ted Yu <yuzhih...@gmail.com>
Subject Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Date Fri, 01 Dec 2017 17:38:01 GMT
Thanks for the update.

bq. To enable this, the version of the broker will be added to the JSON
registered by each broker during startup at */brokers/ids/id*

*It seems the path has typo. Should it be:*
*/config/brokers/id*

*Cheers*

On Fri, Dec 1, 2017 at 8:32 AM, Rajini Sivaram <rajinisivaram@gmail.com>
wrote:

> Made one change to the KIP - I added a *validate* method to the
> *Reconfigurable* interface so that we can validate new configs before they
> are applied.
>
> A couple of initial implementations:
>
>    1. Dynamic updates of keystores:
>    https://github.com/rajinisivaram/kafka/commit/
> 3071b686973a59a45546e9db6bdb05578d2edc0b
>    2. Metrics reporter reconfiguration:
>    https://github.com/rajinisivaram/kafka/commit/
> 7c0aa1ea1d81273fe6c082d47fff16208885d3df
>
>
> On Fri, Dec 1, 2017 at 4:04 PM, Rajini Sivaram <rajinisivaram@gmail.com>
> wrote:
>
> > Hi all,
> >
> > If there are no other suggestions or comments, I will start vote next
> > week. In the meantime, please feel free to review and add any suggestions
> > or improvements.
> >
> > Regards,
> >
> > Rajini
> >
> > On Wed, Nov 29, 2017 at 11:52 AM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > wrote:
> >
> >> Hi Jason,
> >>
> >> Thanks for reviewing the KIP.
> >>
> >> I hadn't included *inter.broker.protocol.version*, but you have
> provided
> >> a good reason to do that in order to avoid an additional rolling restart
> >> during upgrade. I had included *log.message.format.version* along with
> >> other default topic configs, but it probably makes sense to do these two
> >> together.
> >>
> >>
> >> On Wed, Nov 29, 2017 at 12:00 AM, Jason Gustafson <jason@confluent.io>
> >> wrote:
> >>
> >>> Hi Rajini,
> >>>
> >>> One quick question I was wondering about is whether this could be used
> to
> >>> update the inter-broker protocol version or the message format version?
> >>> Potentially then we'd only need one rolling restart to upgrade the
> >>> cluster.
> >>> I glanced quickly through the uses of this config in the code and it
> >>> seems
> >>> like it might be possible. Not sure if there are any complications you
> or
> >>> others can think of.
> >>>
> >>> Thanks,
> >>> Jason
> >>>
> >>> On Tue, Nov 28, 2017 at 2:48 PM, Rajini Sivaram <
> rajinisivaram@gmail.com
> >>> >
> >>> 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.
> >>> >
> >>> > 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.
> >>> >
> >>> >
> >>> > 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message