+1 Thanks for the KIP. One minor nit: I think we changed ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the PR.
As far as updating secrets, I wasn't sure I understand how that will work. Do the password configs accept multiple values? On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <rajinisiva...@gmail.com> wrote: > Hi Jun, > > Thank you for reviewing and voting. > > 50. I have updated the KIP to describe how the secret may be changed. All > dynamically configurable passwords and per-broker configs. So the secret > can be different across brokers and updated using rolling restart. In order > to update the secret, each broker needs to be restarted with an updated > server.properties which contains the new secret as well as the current > values of all the password configs. Admin client can then be used to update > the passwords in ZooKeeper that are encrypted using the new secret. > > 51. leader.replication.throttled.replicas and > follower.replication.throttled.replicas > are dynamically configurable at the topic level. But there are no defaults > for these at the broker level since they refer to partitions of the topic. > The rates used for throttling were already configurable at the broker > level. > > I made a couple of other changes to the KIP: > > 1. The config names used for encoding passwords are now prefixed with > password.encoder. > Also added key length as a config since this is constrained by the > algorithm which is also configurable. > 2. I moved the update of inter-broker security protocol and > inter-broker sasl mechanism to the follow-on KIP under Future Work. As part > of the new KIP, we need to add protocol changes to validate that all > brokers in the cluster support the new protocol/mechanism/version to avoid > accidental changes before all brokers are updated. > > > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <j...@confluent.io> wrote: > > > Hi, Rajini, > > > > Thank for the KIP. +1. Just a couple of minor comments below. > > > > > > 50. config.secret.*: Could you document how the encryption/decryption of > > passwd work? In particular, how do we support changing config.secret? > > > > 51. At the topic level, we also have leader.replication.throttled. > replicas > > and follower.replication.throttled.replicas. Should they be dynamically > > configurable? > > > > Jun > > > > > > > > > > > > > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <g...@confluent.io> wrote: > > > > > +1 (binding). Thank you for leading this, Rajini. > > > > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t.j.bent...@gmail.com> > > wrote: > > > > > > > +1 (nonbinding) > > > > > > > > On 12 December 2017 at 15:34, Ted Yu <yuzhih...@gmail.com> wrote: > > > > > > > > > +1 > > > > > > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram < > > > rajinisiva...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > Since there are no more outstanding comments, I would like to > start > > > > vote > > > > > > for KIP-226: > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > 226+-+Dynamic+Broker+Configuration > > > > > > > > > > > > > > > > > > The KIP enables dynamic update of commonly updated broker > > > configuration > > > > > > options to avoid expensive restarts. > > > > > > > > > > > > Thank you, > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > >
