kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rajini Sivaram <rajinisiva...@gmail.com>
Subject Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Date Fri, 01 Dec 2017 16:04:45 GMT
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