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 <rajinisiva...@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 <rajinisiva...@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 < > rajinisiva...@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 <ja...@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 < > rajinisiva...@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 <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 > >>> > > > > > >> > > > > > > >>> > > > > > >> > > > > > >>> > > > > > >> > > > > >>> > > > > > >> > > > >>> > > > > > >> > > >>> > > > > > >> > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > >>> > > >>> > >> > >> > > >
