Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi all, I made a change to the KIP to specify sasl.jaas.config property for brokers separately for each mechanism, rather than together in a single login context as we do today in the JAAS config file. This will make it easier to add new mechanisms to a running broker (this is not in scope for this KIP, but we might want to do in future). Please let me know if you have any concerns. Thank you, Rajini On Thu, Jan 25, 2018 at 3:51 AM, Viktor Somogyi wrote: > Yea, if other commands seem to follow this pattern, I'll update KIP-248 as > well :). Also introducing those arguments in the current ConfigCommand also > makes sense from the migration point of view too as it will be introduced > in 1.1 which makes it somewhat easier for KIP-248. > > On Wed, Jan 24, 2018 at 6:46 PM, Rajini Sivaram > wrote: > > > Hi Ismael, > > > > Yes, that makes sense. Looking at the command line options for different > > tools, we seem to be using *--command-config *in the > commands > > that currently talk to the new AdminClient (DelegationTokenCommand, > > ConsumerGroupCommand, DeleteRecordsCommand). So perhaps it makes sense to > > do the same for ConfigCommand as well. I will update KIP-226 with the two > > options *--bootstrap-server* and *--command-config*. > > > > Viktor, what do you think? > > > > At the moment, I think many in the community are busy due to the code > > freeze next week, but hopefully we should get more feedback on KIP-248 > soon > > after. > > > > Thank you, > > > > Rajini > > > > On Wed, Jan 24, 2018 at 5:41 AM, Viktor Somogyi > > > wrote: > > > > > Hi all, > > > > > > I'd also like to as the community here who were participating the > > > discussion of KIP-226 to take a look at KIP-248 (that is making > > > kafka-configs.sh fully function with AdminClient and a Java based > > > ConfigCommand). It would be much appreciated to get feedback on that as > > it > > > plays an important role for KIP-226 and other long-waited features. > > > > > > Thanks, > > > Viktor > > > > > > On Wed, Jan 24, 2018 at 6:56 AM, Ismael Juma > wrote: > > > > > > > Hi Rajini, > > > > > > > > I think the proposal makes sense. One suggestion: can we just allow > the > > > > config to be passed? That is, leave out the properties config for > now. > > > > > > > > On Tue, Jan 23, 2018 at 3:01 PM, Rajini Sivaram < > > rajinisiva...@gmail.com > > > > > > > > wrote: > > > > > > > > > Since we are running out of time to get the whole ConfigCommand > > > converted > > > > > to using the new AdminClient for 1.1.0 (KIP-248), we need a way to > > > enable > > > > > ConfigCommand to handle broker config updates (implemented by > > KIP-226). > > > > As > > > > > a simple first step, it would make sense to use the existing > > > > ConfigCommand > > > > > tool to perform broker config updates enabled by this KIP. Since > > config > > > > > validation and password encryption are performed by the broker, > this > > > will > > > > > be easier to do with the new AdminClient. To do this, we need to > add > > > > > command line options for new admin client to kafka-configs.sh. > > Dynamic > > > > > broker config updates alone will be done under KIP-226 using the > new > > > > admin > > > > > client to make this feature usable.. The new command line options > > > > > (consistent with KIP-248) that will be added to ConfigCommand will > > be: > > > > > > > > > >- --bootstrap-server *host:port* > > > > >- --adminclient.config *config-file* > > > > >- --adminclient.properties *k1=v1,k2=v2* > > > > > > > > > > If anyone has any concerns about these options being added to > > > > > kafka-configs.sh, please let me know. Otherwise, I will update > > KIP-226 > > > > and > > > > > add the options to one of the KIP-226 PRs. > > > > > > > > > > Thank you, > > > > > > > > > > Rajini > > > > > > > > > > On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma > > > wrote: > > > > > > > > > > > Thanks Rajini. Sounds good. > > > > > > > > > > > > Ismael > > > > > > > > > > > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram < > > > > > rajinisiva...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > Hi Ismael, > > > > > > > > > > > > > > I have updated the KIP to use AES-256 if available and AES-128 > > > > > otherwise > > > > > > > for password encryption. Looking at GCM, it looks like GCM is > > > > typically > > > > > > > used with a variable initialization vector, while we are using > a > > > > > random, > > > > > > > but constant IV per-password. Also, AES/GCM is not supported by > > > > Java7. > > > > > > > Since the authentication and performance benefits of GCM are > not > > > > > required > > > > > > > for this scenario, I am thinking I will leave the default as > CBC, > > > but > > > > > > make > > > > > > > sure we test GCM as well so that users have the choice. > > > > > > > > > > > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe < > > cmcc...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > > > Thanks, Rajini. That makes sense.
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Yea, if other commands seem to follow this pattern, I'll update KIP-248 as well :). Also introducing those arguments in the current ConfigCommand also makes sense from the migration point of view too as it will be introduced in 1.1 which makes it somewhat easier for KIP-248. On Wed, Jan 24, 2018 at 6:46 PM, Rajini Sivaram wrote: > Hi Ismael, > > Yes, that makes sense. Looking at the command line options for different > tools, we seem to be using *--command-config *in the commands > that currently talk to the new AdminClient (DelegationTokenCommand, > ConsumerGroupCommand, DeleteRecordsCommand). So perhaps it makes sense to > do the same for ConfigCommand as well. I will update KIP-226 with the two > options *--bootstrap-server* and *--command-config*. > > Viktor, what do you think? > > At the moment, I think many in the community are busy due to the code > freeze next week, but hopefully we should get more feedback on KIP-248 soon > after. > > Thank you, > > Rajini > > On Wed, Jan 24, 2018 at 5:41 AM, Viktor Somogyi > wrote: > > > Hi all, > > > > I'd also like to as the community here who were participating the > > discussion of KIP-226 to take a look at KIP-248 (that is making > > kafka-configs.sh fully function with AdminClient and a Java based > > ConfigCommand). It would be much appreciated to get feedback on that as > it > > plays an important role for KIP-226 and other long-waited features. > > > > Thanks, > > Viktor > > > > On Wed, Jan 24, 2018 at 6:56 AM, Ismael Juma wrote: > > > > > Hi Rajini, > > > > > > I think the proposal makes sense. One suggestion: can we just allow the > > > config to be passed? That is, leave out the properties config for now. > > > > > > On Tue, Jan 23, 2018 at 3:01 PM, Rajini Sivaram < > rajinisiva...@gmail.com > > > > > > wrote: > > > > > > > Since we are running out of time to get the whole ConfigCommand > > converted > > > > to using the new AdminClient for 1.1.0 (KIP-248), we need a way to > > enable > > > > ConfigCommand to handle broker config updates (implemented by > KIP-226). > > > As > > > > a simple first step, it would make sense to use the existing > > > ConfigCommand > > > > tool to perform broker config updates enabled by this KIP. Since > config > > > > validation and password encryption are performed by the broker, this > > will > > > > be easier to do with the new AdminClient. To do this, we need to add > > > > command line options for new admin client to kafka-configs.sh. > Dynamic > > > > broker config updates alone will be done under KIP-226 using the new > > > admin > > > > client to make this feature usable.. The new command line options > > > > (consistent with KIP-248) that will be added to ConfigCommand will > be: > > > > > > > >- --bootstrap-server *host:port* > > > >- --adminclient.config *config-file* > > > >- --adminclient.properties *k1=v1,k2=v2* > > > > > > > > If anyone has any concerns about these options being added to > > > > kafka-configs.sh, please let me know. Otherwise, I will update > KIP-226 > > > and > > > > add the options to one of the KIP-226 PRs. > > > > > > > > Thank you, > > > > > > > > Rajini > > > > > > > > On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma > > wrote: > > > > > > > > > Thanks Rajini. Sounds good. > > > > > > > > > > Ismael > > > > > > > > > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram < > > > > rajinisiva...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hi Ismael, > > > > > > > > > > > > I have updated the KIP to use AES-256 if available and AES-128 > > > > otherwise > > > > > > for password encryption. Looking at GCM, it looks like GCM is > > > typically > > > > > > used with a variable initialization vector, while we are using a > > > > random, > > > > > > but constant IV per-password. Also, AES/GCM is not supported by > > > Java7. > > > > > > Since the authentication and performance benefits of GCM are not > > > > required > > > > > > for this scenario, I am thinking I will leave the default as CBC, > > but > > > > > make > > > > > > sure we test GCM as well so that users have the choice. > > > > > > > > > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe < > cmcc...@apache.org> > > > > > wrote: > > > > > > > > > > > > > Thanks, Rajini. That makes sense. > > > > > > > > > > > > > > regards, > > > > > > > Colin > > > > > > > > > > > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > > > > > > Hi Colin, > > > > > > > > > > > > > > > > Thank you for reviewing. > > > > > > > > > > > > > > > > Yes, validation is done on the broker, not the client. > > > > > > > > > > > > > > > > All configs from ZooKeeper are processed and any config that > > > could > > > > > not > > > > > > be > > > > > > > > applied are logged as warnings. This includes any configs > that > > > are > > > > > not > > > > > > > > dynamic in the broker version or any configs that are not > > > supported > > > > > in > > > > > > > the > > > > > > > > broker version. If you downgrade to a version that
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Ismael, Yes, that makes sense. Looking at the command line options for different tools, we seem to be using *--command-config *in the commands that currently talk to the new AdminClient (DelegationTokenCommand, ConsumerGroupCommand, DeleteRecordsCommand). So perhaps it makes sense to do the same for ConfigCommand as well. I will update KIP-226 with the two options *--bootstrap-server* and *--command-config*. Viktor, what do you think? At the moment, I think many in the community are busy due to the code freeze next week, but hopefully we should get more feedback on KIP-248 soon after. Thank you, Rajini On Wed, Jan 24, 2018 at 5:41 AM, Viktor Somogyi wrote: > Hi all, > > I'd also like to as the community here who were participating the > discussion of KIP-226 to take a look at KIP-248 (that is making > kafka-configs.sh fully function with AdminClient and a Java based > ConfigCommand). It would be much appreciated to get feedback on that as it > plays an important role for KIP-226 and other long-waited features. > > Thanks, > Viktor > > On Wed, Jan 24, 2018 at 6:56 AM, Ismael Juma wrote: > > > Hi Rajini, > > > > I think the proposal makes sense. One suggestion: can we just allow the > > config to be passed? That is, leave out the properties config for now. > > > > On Tue, Jan 23, 2018 at 3:01 PM, Rajini Sivaram > > > wrote: > > > > > Since we are running out of time to get the whole ConfigCommand > converted > > > to using the new AdminClient for 1.1.0 (KIP-248), we need a way to > enable > > > ConfigCommand to handle broker config updates (implemented by KIP-226). > > As > > > a simple first step, it would make sense to use the existing > > ConfigCommand > > > tool to perform broker config updates enabled by this KIP. Since config > > > validation and password encryption are performed by the broker, this > will > > > be easier to do with the new AdminClient. To do this, we need to add > > > command line options for new admin client to kafka-configs.sh. Dynamic > > > broker config updates alone will be done under KIP-226 using the new > > admin > > > client to make this feature usable.. The new command line options > > > (consistent with KIP-248) that will be added to ConfigCommand will be: > > > > > >- --bootstrap-server *host:port* > > >- --adminclient.config *config-file* > > >- --adminclient.properties *k1=v1,k2=v2* > > > > > > If anyone has any concerns about these options being added to > > > kafka-configs.sh, please let me know. Otherwise, I will update KIP-226 > > and > > > add the options to one of the KIP-226 PRs. > > > > > > Thank you, > > > > > > Rajini > > > > > > On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma > wrote: > > > > > > > Thanks Rajini. Sounds good. > > > > > > > > Ismael > > > > > > > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram < > > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Ismael, > > > > > > > > > > I have updated the KIP to use AES-256 if available and AES-128 > > > otherwise > > > > > for password encryption. Looking at GCM, it looks like GCM is > > typically > > > > > used with a variable initialization vector, while we are using a > > > random, > > > > > but constant IV per-password. Also, AES/GCM is not supported by > > Java7. > > > > > Since the authentication and performance benefits of GCM are not > > > required > > > > > for this scenario, I am thinking I will leave the default as CBC, > but > > > > make > > > > > sure we test GCM as well so that users have the choice. > > > > > > > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe > > > > wrote: > > > > > > > > > > > Thanks, Rajini. That makes sense. > > > > > > > > > > > > regards, > > > > > > Colin > > > > > > > > > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > > > > > Hi Colin, > > > > > > > > > > > > > > Thank you for reviewing. > > > > > > > > > > > > > > Yes, validation is done on the broker, not the client. > > > > > > > > > > > > > > All configs from ZooKeeper are processed and any config that > > could > > > > not > > > > > be > > > > > > > applied are logged as warnings. This includes any configs that > > are > > > > not > > > > > > > dynamic in the broker version or any configs that are not > > supported > > > > in > > > > > > the > > > > > > > broker version. If you downgrade to a version that is older > than > > > this > > > > > KIP > > > > > > > (1.0 for example), then you don't get any warnings however. > > > > > > > > > > > > > > > > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe < > cmcc...@apache.org > > > > > > > > wrote: > > > > > > > > > > > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > > > > > > Hi Rajini, > > > > > > > > > > > > > > > > > > Looking good. Just a few questions. > > > > > > > > > > > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > > > > > > Reconfigurable > > > > > > > > > necessary? I would have thought we'd validate using the > > > > ConfigDef. > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi all, I'd also like to as the community here who were participating the discussion of KIP-226 to take a look at KIP-248 (that is making kafka-configs.sh fully function with AdminClient and a Java based ConfigCommand). It would be much appreciated to get feedback on that as it plays an important role for KIP-226 and other long-waited features. Thanks, Viktor On Wed, Jan 24, 2018 at 6:56 AM, Ismael Juma wrote: > Hi Rajini, > > I think the proposal makes sense. One suggestion: can we just allow the > config to be passed? That is, leave out the properties config for now. > > On Tue, Jan 23, 2018 at 3:01 PM, Rajini Sivaram > wrote: > > > Since we are running out of time to get the whole ConfigCommand converted > > to using the new AdminClient for 1.1.0 (KIP-248), we need a way to enable > > ConfigCommand to handle broker config updates (implemented by KIP-226). > As > > a simple first step, it would make sense to use the existing > ConfigCommand > > tool to perform broker config updates enabled by this KIP. Since config > > validation and password encryption are performed by the broker, this will > > be easier to do with the new AdminClient. To do this, we need to add > > command line options for new admin client to kafka-configs.sh. Dynamic > > broker config updates alone will be done under KIP-226 using the new > admin > > client to make this feature usable.. The new command line options > > (consistent with KIP-248) that will be added to ConfigCommand will be: > > > >- --bootstrap-server *host:port* > >- --adminclient.config *config-file* > >- --adminclient.properties *k1=v1,k2=v2* > > > > If anyone has any concerns about these options being added to > > kafka-configs.sh, please let me know. Otherwise, I will update KIP-226 > and > > add the options to one of the KIP-226 PRs. > > > > Thank you, > > > > Rajini > > > > On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma wrote: > > > > > Thanks Rajini. Sounds good. > > > > > > Ismael > > > > > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > wrote: > > > > > > > Hi Ismael, > > > > > > > > I have updated the KIP to use AES-256 if available and AES-128 > > otherwise > > > > for password encryption. Looking at GCM, it looks like GCM is > typically > > > > used with a variable initialization vector, while we are using a > > random, > > > > but constant IV per-password. Also, AES/GCM is not supported by > Java7. > > > > Since the authentication and performance benefits of GCM are not > > required > > > > for this scenario, I am thinking I will leave the default as CBC, but > > > make > > > > sure we test GCM as well so that users have the choice. > > > > > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe > > > wrote: > > > > > > > > > Thanks, Rajini. That makes sense. > > > > > > > > > > regards, > > > > > Colin > > > > > > > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > > > > Hi Colin, > > > > > > > > > > > > Thank you for reviewing. > > > > > > > > > > > > Yes, validation is done on the broker, not the client. > > > > > > > > > > > > All configs from ZooKeeper are processed and any config that > could > > > not > > > > be > > > > > > applied are logged as warnings. This includes any configs that > are > > > not > > > > > > dynamic in the broker version or any configs that are not > supported > > > in > > > > > the > > > > > > broker version. If you downgrade to a version that is older than > > this > > > > KIP > > > > > > (1.0 for example), then you don't get any warnings however. > > > > > > > > > > > > > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe > > > > > wrote: > > > > > > > > > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > > > > > Hi Rajini, > > > > > > > > > > > > > > > > Looking good. Just a few questions. > > > > > > > > > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > > > > > Reconfigurable > > > > > > > > necessary? I would have thought we'd validate using the > > > ConfigDef. > > > > > Do you > > > > > > > > have a use case in mind in which the reconfigurable component > > > only > > > > > > > permits > > > > > > > > certain reconfigurations? > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > Sorry if this is a dumb question, but when we talk about > > validating > > > > on > > > > > the > > > > > > > ConfigDef, we're talking about validating on the server side, > > > right? > > > > > The > > > > > > > software on the client side might be older or newer than the > > > software > > > > > on > > > > > > > the broker side, so it seems inadvisable to do the validation > > > there. > > > > > > > > > > > > > > Also, after a software downgrade, when the broker is restarted, > > it > > > > > might > > > > > > > find that there is a configuration key that is stored in ZK > that > > is > > > > not > > > > > > > dynamic in its (older) software version. It seems like, with > the > > > > > current > > > > > > > propo
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Rajini, I think the proposal makes sense. One suggestion: can we just allow the config to be passed? That is, leave out the properties config for now. On Tue, Jan 23, 2018 at 3:01 PM, Rajini Sivaram wrote: > Since we are running out of time to get the whole ConfigCommand converted > to using the new AdminClient for 1.1.0 (KIP-248), we need a way to enable > ConfigCommand to handle broker config updates (implemented by KIP-226). As > a simple first step, it would make sense to use the existing ConfigCommand > tool to perform broker config updates enabled by this KIP. Since config > validation and password encryption are performed by the broker, this will > be easier to do with the new AdminClient. To do this, we need to add > command line options for new admin client to kafka-configs.sh. Dynamic > broker config updates alone will be done under KIP-226 using the new admin > client to make this feature usable.. The new command line options > (consistent with KIP-248) that will be added to ConfigCommand will be: > >- --bootstrap-server *host:port* >- --adminclient.config *config-file* >- --adminclient.properties *k1=v1,k2=v2* > > If anyone has any concerns about these options being added to > kafka-configs.sh, please let me know. Otherwise, I will update KIP-226 and > add the options to one of the KIP-226 PRs. > > Thank you, > > Rajini > > On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma wrote: > > > Thanks Rajini. Sounds good. > > > > Ismael > > > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram < > rajinisiva...@gmail.com> > > wrote: > > > > > Hi Ismael, > > > > > > I have updated the KIP to use AES-256 if available and AES-128 > otherwise > > > for password encryption. Looking at GCM, it looks like GCM is typically > > > used with a variable initialization vector, while we are using a > random, > > > but constant IV per-password. Also, AES/GCM is not supported by Java7. > > > Since the authentication and performance benefits of GCM are not > required > > > for this scenario, I am thinking I will leave the default as CBC, but > > make > > > sure we test GCM as well so that users have the choice. > > > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe > > wrote: > > > > > > > Thanks, Rajini. That makes sense. > > > > > > > > regards, > > > > Colin > > > > > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > > > Hi Colin, > > > > > > > > > > Thank you for reviewing. > > > > > > > > > > Yes, validation is done on the broker, not the client. > > > > > > > > > > All configs from ZooKeeper are processed and any config that could > > not > > > be > > > > > applied are logged as warnings. This includes any configs that are > > not > > > > > dynamic in the broker version or any configs that are not supported > > in > > > > the > > > > > broker version. If you downgrade to a version that is older than > this > > > KIP > > > > > (1.0 for example), then you don't get any warnings however. > > > > > > > > > > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe > > > wrote: > > > > > > > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > > > > Hi Rajini, > > > > > > > > > > > > > > Looking good. Just a few questions. > > > > > > > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > > > > Reconfigurable > > > > > > > necessary? I would have thought we'd validate using the > > ConfigDef. > > > > Do you > > > > > > > have a use case in mind in which the reconfigurable component > > only > > > > > > permits > > > > > > > certain reconfigurations? > > > > > > > > > > > > Hi, > > > > > > > > > > > > Sorry if this is a dumb question, but when we talk about > validating > > > on > > > > the > > > > > > ConfigDef, we're talking about validating on the server side, > > right? > > > > The > > > > > > software on the client side might be older or newer than the > > software > > > > on > > > > > > the broker side, so it seems inadvisable to do the validation > > there. > > > > > > > > > > > > Also, after a software downgrade, when the broker is restarted, > it > > > > might > > > > > > find that there is a configuration key that is stored in ZK that > is > > > not > > > > > > dynamic in its (older) software version. It seems like, with the > > > > current > > > > > > proposal, the broker will use the value found in the local > > > > configuration > > > > > > (config file) rather than the new ZK version. Should the broker > > > print > > > > out > > > > > > a WARN message in that scenario? > > > > > > > > > > > > best, > > > > > > Colin > > > > > > > > > > > > > 2. Should Reconfigurable extend Configurable or is the initial > > > > > > > configuration also done through reconfigure()? I ask because > not > > > all > > > > > > > plugins interfaces currently extend Configurable (e.g. > > > > > > > KafkaPrincipalBuilder). > > > > > > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > > > > > > DescribeConfigsResult. Perhaps we should list
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Since we are running out of time to get the whole ConfigCommand converted to using the new AdminClient for 1.1.0 (KIP-248), we need a way to enable ConfigCommand to handle broker config updates (implemented by KIP-226). As a simple first step, it would make sense to use the existing ConfigCommand tool to perform broker config updates enabled by this KIP. Since config validation and password encryption are performed by the broker, this will be easier to do with the new AdminClient. To do this, we need to add command line options for new admin client to kafka-configs.sh. Dynamic broker config updates alone will be done under KIP-226 using the new admin client to make this feature usable.. The new command line options (consistent with KIP-248) that will be added to ConfigCommand will be: - --bootstrap-server *host:port* - --adminclient.config *config-file* - --adminclient.properties *k1=v1,k2=v2* If anyone has any concerns about these options being added to kafka-configs.sh, please let me know. Otherwise, I will update KIP-226 and add the options to one of the KIP-226 PRs. Thank you, Rajini On Wed, Jan 10, 2018 at 5:14 AM, Ismael Juma wrote: > Thanks Rajini. Sounds good. > > Ismael > > On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram > wrote: > > > Hi Ismael, > > > > I have updated the KIP to use AES-256 if available and AES-128 otherwise > > for password encryption. Looking at GCM, it looks like GCM is typically > > used with a variable initialization vector, while we are using a random, > > but constant IV per-password. Also, AES/GCM is not supported by Java7. > > Since the authentication and performance benefits of GCM are not required > > for this scenario, I am thinking I will leave the default as CBC, but > make > > sure we test GCM as well so that users have the choice. > > > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe > wrote: > > > > > Thanks, Rajini. That makes sense. > > > > > > regards, > > > Colin > > > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > > Hi Colin, > > > > > > > > Thank you for reviewing. > > > > > > > > Yes, validation is done on the broker, not the client. > > > > > > > > All configs from ZooKeeper are processed and any config that could > not > > be > > > > applied are logged as warnings. This includes any configs that are > not > > > > dynamic in the broker version or any configs that are not supported > in > > > the > > > > broker version. If you downgrade to a version that is older than this > > KIP > > > > (1.0 for example), then you don't get any warnings however. > > > > > > > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe > > wrote: > > > > > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > > > Hi Rajini, > > > > > > > > > > > > Looking good. Just a few questions. > > > > > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > > > Reconfigurable > > > > > > necessary? I would have thought we'd validate using the > ConfigDef. > > > Do you > > > > > > have a use case in mind in which the reconfigurable component > only > > > > > permits > > > > > > certain reconfigurations? > > > > > > > > > > Hi, > > > > > > > > > > Sorry if this is a dumb question, but when we talk about validating > > on > > > the > > > > > ConfigDef, we're talking about validating on the server side, > right? > > > The > > > > > software on the client side might be older or newer than the > software > > > on > > > > > the broker side, so it seems inadvisable to do the validation > there. > > > > > > > > > > Also, after a software downgrade, when the broker is restarted, it > > > might > > > > > find that there is a configuration key that is stored in ZK that is > > not > > > > > dynamic in its (older) software version. It seems like, with the > > > current > > > > > proposal, the broker will use the value found in the local > > > configuration > > > > > (config file) rather than the new ZK version. Should the broker > > print > > > out > > > > > a WARN message in that scenario? > > > > > > > > > > best, > > > > > Colin > > > > > > > > > > > 2. Should Reconfigurable extend Configurable or is the initial > > > > > > configuration also done through reconfigure()? I ask because not > > all > > > > > > plugins interfaces currently extend Configurable (e.g. > > > > > > KafkaPrincipalBuilder). > > > > > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > > > > > DescribeConfigsResult. Perhaps we should list the changes > > > explicitly? One > > > > > > not totally obvious case is what the synonyms() getter would > return > > > if > > > > > the > > > > > > option is not specified (i.e. should it raise an exception or > > return > > > an > > > > > > empty list?). > > > > > > 4. Config entries in the DescribeConfigs response have an > > is_default > > > > > flag. > > > > > > Could that be replaced with the more general config_source? > > > > > > 5. Bit of an internal question, but how do you handle config
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Thanks Rajini. Sounds good. Ismael On Wed, Jan 10, 2018 at 11:41 AM, Rajini Sivaram wrote: > Hi Ismael, > > I have updated the KIP to use AES-256 if available and AES-128 otherwise > for password encryption. Looking at GCM, it looks like GCM is typically > used with a variable initialization vector, while we are using a random, > but constant IV per-password. Also, AES/GCM is not supported by Java7. > Since the authentication and performance benefits of GCM are not required > for this scenario, I am thinking I will leave the default as CBC, but make > sure we test GCM as well so that users have the choice. > > On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe wrote: > > > Thanks, Rajini. That makes sense. > > > > regards, > > Colin > > > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > > Hi Colin, > > > > > > Thank you for reviewing. > > > > > > Yes, validation is done on the broker, not the client. > > > > > > All configs from ZooKeeper are processed and any config that could not > be > > > applied are logged as warnings. This includes any configs that are not > > > dynamic in the broker version or any configs that are not supported in > > the > > > broker version. If you downgrade to a version that is older than this > KIP > > > (1.0 for example), then you don't get any warnings however. > > > > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe > wrote: > > > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > > Hi Rajini, > > > > > > > > > > Looking good. Just a few questions. > > > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > > Reconfigurable > > > > > necessary? I would have thought we'd validate using the ConfigDef. > > Do you > > > > > have a use case in mind in which the reconfigurable component only > > > > permits > > > > > certain reconfigurations? > > > > > > > > Hi, > > > > > > > > Sorry if this is a dumb question, but when we talk about validating > on > > the > > > > ConfigDef, we're talking about validating on the server side, right? > > The > > > > software on the client side might be older or newer than the software > > on > > > > the broker side, so it seems inadvisable to do the validation there. > > > > > > > > Also, after a software downgrade, when the broker is restarted, it > > might > > > > find that there is a configuration key that is stored in ZK that is > not > > > > dynamic in its (older) software version. It seems like, with the > > current > > > > proposal, the broker will use the value found in the local > > configuration > > > > (config file) rather than the new ZK version. Should the broker > print > > out > > > > a WARN message in that scenario? > > > > > > > > best, > > > > Colin > > > > > > > > > 2. Should Reconfigurable extend Configurable or is the initial > > > > > configuration also done through reconfigure()? I ask because not > all > > > > > plugins interfaces currently extend Configurable (e.g. > > > > > KafkaPrincipalBuilder). > > > > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > > > > DescribeConfigsResult. Perhaps we should list the changes > > explicitly? One > > > > > not totally obvious case is what the synonyms() getter would return > > if > > > > the > > > > > option is not specified (i.e. should it raise an exception or > return > > an > > > > > empty list?). > > > > > 4. Config entries in the DescribeConfigs response have an > is_default > > > > flag. > > > > > Could that be replaced with the more general config_source? > > > > > 5. Bit of an internal question, but how do you handle config > > > > dependencies? > > > > > For example, suppose I want to add a listener and configure its > > principal > > > > > builder at once. You'd have to validate the principal builder > config > > in > > > > the > > > > > context of the listener config, so I guess the order of the entries > > in > > > > > AlterConfigs is significant? > > > > > 6. KIP-48 (delegation tokens) gives us a master secret which is > > shared by > > > > > all brokers. Do you think we would make this dynamically > > configurable? > > > > > Alternatively, it might be possible to use it to encrypt the other > > > > > passwords we store in zookeeper. > > > > > > > > > > Thanks, > > > > > Jason > > > > > > > > > > > > > > > > > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram < > > > > rajinisiva...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hi Jay, > > > > > > > > > > > > Thank you for reviewing the KIP. > > > > > > > > > > > > 1) Yes, makes sense. I will update the PR. There are some config > > > > updates > > > > > > that may be allowed depending on the context (e.g. some security > > > > configs > > > > > > can be updated for new listeners, but not existing listeners). > > Perhaps > > > > it > > > > > > is ok to mark them dynamic in the documentation. AdminClient > would > > give > > > > > > appropriate error messages if the update is not allowed. > > > > > > 2) Internally, in the implementat
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Ismael, I have updated the KIP to use AES-256 if available and AES-128 otherwise for password encryption. Looking at GCM, it looks like GCM is typically used with a variable initialization vector, while we are using a random, but constant IV per-password. Also, AES/GCM is not supported by Java7. Since the authentication and performance benefits of GCM are not required for this scenario, I am thinking I will leave the default as CBC, but make sure we test GCM as well so that users have the choice. On Wed, Jan 10, 2018 at 1:01 AM, Colin McCabe wrote: > Thanks, Rajini. That makes sense. > > regards, > Colin > > On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > > Hi Colin, > > > > Thank you for reviewing. > > > > Yes, validation is done on the broker, not the client. > > > > All configs from ZooKeeper are processed and any config that could not be > > applied are logged as warnings. This includes any configs that are not > > dynamic in the broker version or any configs that are not supported in > the > > broker version. If you downgrade to a version that is older than this KIP > > (1.0 for example), then you don't get any warnings however. > > > > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe wrote: > > > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > > Hi Rajini, > > > > > > > > Looking good. Just a few questions. > > > > > > > > 1. (Related to Jay's comment) Is the validate() method on > Reconfigurable > > > > necessary? I would have thought we'd validate using the ConfigDef. > Do you > > > > have a use case in mind in which the reconfigurable component only > > > permits > > > > certain reconfigurations? > > > > > > Hi, > > > > > > Sorry if this is a dumb question, but when we talk about validating on > the > > > ConfigDef, we're talking about validating on the server side, right? > The > > > software on the client side might be older or newer than the software > on > > > the broker side, so it seems inadvisable to do the validation there. > > > > > > Also, after a software downgrade, when the broker is restarted, it > might > > > find that there is a configuration key that is stored in ZK that is not > > > dynamic in its (older) software version. It seems like, with the > current > > > proposal, the broker will use the value found in the local > configuration > > > (config file) rather than the new ZK version. Should the broker print > out > > > a WARN message in that scenario? > > > > > > best, > > > Colin > > > > > > > 2. Should Reconfigurable extend Configurable or is the initial > > > > configuration also done through reconfigure()? I ask because not all > > > > plugins interfaces currently extend Configurable (e.g. > > > > KafkaPrincipalBuilder). > > > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > > > DescribeConfigsResult. Perhaps we should list the changes > explicitly? One > > > > not totally obvious case is what the synonyms() getter would return > if > > > the > > > > option is not specified (i.e. should it raise an exception or return > an > > > > empty list?). > > > > 4. Config entries in the DescribeConfigs response have an is_default > > > flag. > > > > Could that be replaced with the more general config_source? > > > > 5. Bit of an internal question, but how do you handle config > > > dependencies? > > > > For example, suppose I want to add a listener and configure its > principal > > > > builder at once. You'd have to validate the principal builder config > in > > > the > > > > context of the listener config, so I guess the order of the entries > in > > > > AlterConfigs is significant? > > > > 6. KIP-48 (delegation tokens) gives us a master secret which is > shared by > > > > all brokers. Do you think we would make this dynamically > configurable? > > > > Alternatively, it might be possible to use it to encrypt the other > > > > passwords we store in zookeeper. > > > > > > > > Thanks, > > > > Jason > > > > > > > > > > > > > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram < > > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Jay, > > > > > > > > > > Thank you for reviewing the KIP. > > > > > > > > > > 1) Yes, makes sense. I will update the PR. There are some config > > > updates > > > > > that may be allowed depending on the context (e.g. some security > > > configs > > > > > can be updated for new listeners, but not existing listeners). > Perhaps > > > it > > > > > is ok to mark them dynamic in the documentation. AdminClient would > give > > > > > appropriate error messages if the update is not allowed. > > > > > 2) Internally, in the implementation, a mixture of direct config > > > updates > > > > > (e.g log config as you have pointed out) and reconfigure method > > > invocations > > > > > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > > > > > reporter), we require the Reconfigurable interface to ensure that > we > > > can > > > > > validate any custom configs and avoid reconfiguration
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Thanks, Rajini. That makes sense. regards, Colin On Tue, Jan 9, 2018, at 14:38, Rajini Sivaram wrote: > Hi Colin, > > Thank you for reviewing. > > Yes, validation is done on the broker, not the client. > > All configs from ZooKeeper are processed and any config that could not be > applied are logged as warnings. This includes any configs that are not > dynamic in the broker version or any configs that are not supported in the > broker version. If you downgrade to a version that is older than this KIP > (1.0 for example), then you don't get any warnings however. > > > On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe wrote: > > > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > > Hi Rajini, > > > > > > Looking good. Just a few questions. > > > > > > 1. (Related to Jay's comment) Is the validate() method on Reconfigurable > > > necessary? I would have thought we'd validate using the ConfigDef. Do you > > > have a use case in mind in which the reconfigurable component only > > permits > > > certain reconfigurations? > > > > Hi, > > > > Sorry if this is a dumb question, but when we talk about validating on the > > ConfigDef, we're talking about validating on the server side, right? The > > software on the client side might be older or newer than the software on > > the broker side, so it seems inadvisable to do the validation there. > > > > Also, after a software downgrade, when the broker is restarted, it might > > find that there is a configuration key that is stored in ZK that is not > > dynamic in its (older) software version. It seems like, with the current > > proposal, the broker will use the value found in the local configuration > > (config file) rather than the new ZK version. Should the broker print out > > a WARN message in that scenario? > > > > best, > > Colin > > > > > 2. Should Reconfigurable extend Configurable or is the initial > > > configuration also done through reconfigure()? I ask because not all > > > plugins interfaces currently extend Configurable (e.g. > > > KafkaPrincipalBuilder). > > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > > DescribeConfigsResult. Perhaps we should list the changes explicitly? One > > > not totally obvious case is what the synonyms() getter would return if > > the > > > option is not specified (i.e. should it raise an exception or return an > > > empty list?). > > > 4. Config entries in the DescribeConfigs response have an is_default > > flag. > > > Could that be replaced with the more general config_source? > > > 5. Bit of an internal question, but how do you handle config > > dependencies? > > > For example, suppose I want to add a listener and configure its principal > > > builder at once. You'd have to validate the principal builder config in > > the > > > context of the listener config, so I guess the order of the entries in > > > AlterConfigs is significant? > > > 6. KIP-48 (delegation tokens) gives us a master secret which is shared by > > > all brokers. Do you think we would make this dynamically configurable? > > > Alternatively, it might be possible to use it to encrypt the other > > > passwords we store in zookeeper. > > > > > > Thanks, > > > Jason > > > > > > > > > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > wrote: > > > > > > > Hi Jay, > > > > > > > > Thank you for reviewing the KIP. > > > > > > > > 1) Yes, makes sense. I will update the PR. There are some config > > updates > > > > that may be allowed depending on the context (e.g. some security > > configs > > > > can be updated for new listeners, but not existing listeners). Perhaps > > it > > > > is ok to mark them dynamic in the documentation. AdminClient would give > > > > appropriate error messages if the update is not allowed. > > > > 2) Internally, in the implementation, a mixture of direct config > > updates > > > > (e.g log config as you have pointed out) and reconfigure method > > invocations > > > > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > > > > reporter), we require the Reconfigurable interface to ensure that we > > can > > > > validate any custom configs and avoid reconfiguration for plugin > > versions > > > > that don't support it. > > > > > > > > > > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > > > > > > > > > Two thoughts on implementation (shouldn't effect the KIP): > > > > > > > > > >1. It might be nice to add a parameter to ConfigDef which says > > > > whether a > > > > >configuration is dynamically updatable or not so that we can give > > > > error > > > > >messages if it isn't and also have it reflected in the > > auto-generated > > > > > docs. > > > > >2. For many systems they don't really need to take action if a > > config > > > > >changes, they just need to use the new value. Changing them all to > > > > >Reconfigurable requires managing a fair amount of mutability in > > each > > > > > class > > > > >that accepts
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Colin, Thank you for reviewing. Yes, validation is done on the broker, not the client. All configs from ZooKeeper are processed and any config that could not be applied are logged as warnings. This includes any configs that are not dynamic in the broker version or any configs that are not supported in the broker version. If you downgrade to a version that is older than this KIP (1.0 for example), then you don't get any warnings however. On Tue, Jan 9, 2018 at 9:38 PM, Colin McCabe wrote: > On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > > Hi Rajini, > > > > Looking good. Just a few questions. > > > > 1. (Related to Jay's comment) Is the validate() method on Reconfigurable > > necessary? I would have thought we'd validate using the ConfigDef. Do you > > have a use case in mind in which the reconfigurable component only > permits > > certain reconfigurations? > > Hi, > > Sorry if this is a dumb question, but when we talk about validating on the > ConfigDef, we're talking about validating on the server side, right? The > software on the client side might be older or newer than the software on > the broker side, so it seems inadvisable to do the validation there. > > Also, after a software downgrade, when the broker is restarted, it might > find that there is a configuration key that is stored in ZK that is not > dynamic in its (older) software version. It seems like, with the current > proposal, the broker will use the value found in the local configuration > (config file) rather than the new ZK version. Should the broker print out > a WARN message in that scenario? > > best, > Colin > > > 2. Should Reconfigurable extend Configurable or is the initial > > configuration also done through reconfigure()? I ask because not all > > plugins interfaces currently extend Configurable (e.g. > > KafkaPrincipalBuilder). > > 3. You mentioned a couple changes to DescribeConfigsOptions and > > DescribeConfigsResult. Perhaps we should list the changes explicitly? One > > not totally obvious case is what the synonyms() getter would return if > the > > option is not specified (i.e. should it raise an exception or return an > > empty list?). > > 4. Config entries in the DescribeConfigs response have an is_default > flag. > > Could that be replaced with the more general config_source? > > 5. Bit of an internal question, but how do you handle config > dependencies? > > For example, suppose I want to add a listener and configure its principal > > builder at once. You'd have to validate the principal builder config in > the > > context of the listener config, so I guess the order of the entries in > > AlterConfigs is significant? > > 6. KIP-48 (delegation tokens) gives us a master secret which is shared by > > all brokers. Do you think we would make this dynamically configurable? > > Alternatively, it might be possible to use it to encrypt the other > > passwords we store in zookeeper. > > > > Thanks, > > Jason > > > > > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram < > rajinisiva...@gmail.com> > > wrote: > > > > > Hi Jay, > > > > > > Thank you for reviewing the KIP. > > > > > > 1) Yes, makes sense. I will update the PR. There are some config > updates > > > that may be allowed depending on the context (e.g. some security > configs > > > can be updated for new listeners, but not existing listeners). Perhaps > it > > > is ok to mark them dynamic in the documentation. AdminClient would give > > > appropriate error messages if the update is not allowed. > > > 2) Internally, in the implementation, a mixture of direct config > updates > > > (e.g log config as you have pointed out) and reconfigure method > invocations > > > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > > > reporter), we require the Reconfigurable interface to ensure that we > can > > > validate any custom configs and avoid reconfiguration for plugin > versions > > > that don't support it. > > > > > > > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > > > > > > > Two thoughts on implementation (shouldn't effect the KIP): > > > > > > > >1. It might be nice to add a parameter to ConfigDef which says > > > whether a > > > >configuration is dynamically updatable or not so that we can give > > > error > > > >messages if it isn't and also have it reflected in the > auto-generated > > > > docs. > > > >2. For many systems they don't really need to take action if a > config > > > >changes, they just need to use the new value. Changing them all to > > > >Reconfigurable requires managing a fair amount of mutability in > each > > > > class > > > >that accepts changes. Some need this since they need to take > actions > > > > when a > > > >config changes, but it seems like many just need to update some > value. > > > > For > > > >the later you might just be able to do something like what we do > for > > > >LogConfig where there is a single CurrentConfig instance that has > a > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
On Mon, Dec 18, 2017, at 13:40, Jason Gustafson wrote: > Hi Rajini, > > Looking good. Just a few questions. > > 1. (Related to Jay's comment) Is the validate() method on Reconfigurable > necessary? I would have thought we'd validate using the ConfigDef. Do you > have a use case in mind in which the reconfigurable component only permits > certain reconfigurations? Hi, Sorry if this is a dumb question, but when we talk about validating on the ConfigDef, we're talking about validating on the server side, right? The software on the client side might be older or newer than the software on the broker side, so it seems inadvisable to do the validation there. Also, after a software downgrade, when the broker is restarted, it might find that there is a configuration key that is stored in ZK that is not dynamic in its (older) software version. It seems like, with the current proposal, the broker will use the value found in the local configuration (config file) rather than the new ZK version. Should the broker print out a WARN message in that scenario? best, Colin > 2. Should Reconfigurable extend Configurable or is the initial > configuration also done through reconfigure()? I ask because not all > plugins interfaces currently extend Configurable (e.g. > KafkaPrincipalBuilder). > 3. You mentioned a couple changes to DescribeConfigsOptions and > DescribeConfigsResult. Perhaps we should list the changes explicitly? One > not totally obvious case is what the synonyms() getter would return if the > option is not specified (i.e. should it raise an exception or return an > empty list?). > 4. Config entries in the DescribeConfigs response have an is_default flag. > Could that be replaced with the more general config_source? > 5. Bit of an internal question, but how do you handle config dependencies? > For example, suppose I want to add a listener and configure its principal > builder at once. You'd have to validate the principal builder config in the > context of the listener config, so I guess the order of the entries in > AlterConfigs is significant? > 6. KIP-48 (delegation tokens) gives us a master secret which is shared by > all brokers. Do you think we would make this dynamically configurable? > Alternatively, it might be possible to use it to encrypt the other > passwords we store in zookeeper. > > Thanks, > Jason > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram > wrote: > > > Hi Jay, > > > > Thank you for reviewing the KIP. > > > > 1) Yes, makes sense. I will update the PR. There are some config updates > > that may be allowed depending on the context (e.g. some security configs > > can be updated for new listeners, but not existing listeners). Perhaps it > > is ok to mark them dynamic in the documentation. AdminClient would give > > appropriate error messages if the update is not allowed. > > 2) Internally, in the implementation, a mixture of direct config updates > > (e.g log config as you have pointed out) and reconfigure method invocations > > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > > reporter), we require the Reconfigurable interface to ensure that we can > > validate any custom configs and avoid reconfiguration for plugin versions > > that don't support it. > > > > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > > > > > Two thoughts on implementation (shouldn't effect the KIP): > > > > > >1. It might be nice to add a parameter to ConfigDef which says > > whether a > > >configuration is dynamically updatable or not so that we can give > > error > > >messages if it isn't and also have it reflected in the auto-generated > > > docs. > > >2. For many systems they don't really need to take action if a config > > >changes, they just need to use the new value. Changing them all to > > >Reconfigurable requires managing a fair amount of mutability in each > > > class > > >that accepts changes. Some need this since they need to take actions > > > when a > > >config changes, but it seems like many just need to update some value. > > > For > > >the later you might just be able to do something like what we do for > > >LogConfig where there is a single CurrentConfig instance that has a > > >reference to the current KafkaConfig and always reference your > > > configurable > > >parameters via this (e.g. config.current.myConfig). Dunno if that is > > >actually better, but thought I'd throw it out there. > > > > > > -Jay > > > > > > On Sun, Dec 10, 2017 at 8:09 AM, Rajini Sivaram > > > > > wrote: > > > > > > > Hi Jun, > > > > > > > > Thank you! > > > > > > > > 5. Yes, that makes sense. Agree that we don't want to add protocol > > > changes > > > > to *UpdateMetadataRequest* in this KIP. I have moved the update of > > > > *log.message.format.version* and *inter.broker.protocol.version* to > > > reduce > > > > restarts during upgrade to* Future Work*. We can do this in a follow-on > > > > KIP. > > > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Ismael, Thank you for reviewing the KIP. *password.encoder.iterations: 2048*: That was a mistake in the doc, changed to 4096, which is the minimum we use for SCRAM credentials *password.encoder.key.length: 128: *That is a key size that works with the default cipher algorithm. Will change if we change that. I wasn't sure what to choose for these two, so chose common ones . Lastpass docs say they use *PBKDF2WithHmacSHA256 with AES* - *password.encoder.keyfactory.algorithm: **PBKDF2WithHmacSHAn: *I think PBKDF2 is typically used as the SecretKeyFactory algorithm for password encryption. But not sure if we should choose something different, particularly if we want to support Java7 which doesn't support *PBKDF2WithHmacSHA512*. - password.encoder.cipher.algorithm: *AES/CBC/PKCS5Padding: *I haven't looked at AES/GCM variant, do you know if that is better? On Tue, Jan 9, 2018 at 11:34 AM, Ismael Juma wrote: > Hi Rajini, > > Quick question (sorry if this was already discussed). How were the > following chosen? > > Name: password.encoder.keyfactory.algorithm Type: String Default: > PBKDF2WithHmacSHA512 if available, otherwise PBKDF2WithHmacSHA1 (e.g. > Java7) > Name: password.encoder.cipher.algorithm Type: String Default: > AES/CBC/PKCS5Padding > Name: password.encoder.key.length Type: Integer Default: 128 > Name: password.encoder.iterations Type: Integer Default: 2048 > > Also, was a AES/GCM variant considered as the default cipher algorithm? > > Ismael > > On Mon, Nov 20, 2017 at 1:57 PM, Rajini Sivaram > 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 > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Rajini, Quick question (sorry if this was already discussed). How were the following chosen? Name: password.encoder.keyfactory.algorithm Type: String Default: PBKDF2WithHmacSHA512 if available, otherwise PBKDF2WithHmacSHA1 (e.g. Java7) Name: password.encoder.cipher.algorithm Type: String Default: AES/CBC/PKCS5Padding Name: password.encoder.key.length Type: Integer Default: 128 Name: password.encoder.iterations Type: Integer Default: 2048 Also, was a AES/GCM variant considered as the default cipher algorithm? Ismael On Mon, Nov 20, 2017 at 1:57 PM, Rajini Sivaram 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 >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
4. I wasn't sure what to do, so I left it in there, so that synonyms is a self-contained list. 6. We will never store passwords unencrypted, we will forbid them from being altered if the secret is not configured. Thank you, Rajini On Tue, Dec 19, 2017 at 7:14 PM, Jason Gustafson wrote: > Hi Rajini, > > 4. Changed is_default to config_source in config_entry in the protocol. It > > will be less confusing that way. The method isDefault() will just > > return configSource > > > > Would we still include the active config in the list of synonyms? > > 6. It is a nice idea to have an automatically generated secret to avoid > > yet another config. But I wasn't entirely sure, so went for an explicit > > config instead (a bunch of them actually). I had two concerns (a) we > might > > have a password (like the delegation token master secret) that we want > > to encrypt in future that is stored as a cluster-wide password. It will > be > > better if we can configure the broker secret for that, even though for > > that case we will have the same restriction that all brokers are > configured > > with the same secret. (b) broker writes to meta.properties, so there is a > > possibility of losing the secret. > > > That's fair. I saw it as similar to auto-generation of the broker-id (if > you lose meta.properties, you lose the id also), but maybe it's better to > require an explicit config. If users don't provide a config secret, would > we store passwords unencrypted or would we forbid them from being altered? > > Thanks, > Jason > > > > On Tue, Dec 19, 2017 at 4:16 AM, Rajini Sivaram > wrote: > > > Hi Jason, > > > > Thank you! > > > > 2. Updated the KIP: Reconfigurable extends Configurable > > 4. Changed is_default to config_source in config_entry in the protocol. > It > > will be less confusing that way. The method isDefault() will just > > return configSource > > == DEFAULT_CONFIG. Have also included the changes to the public classes > in > > the KIP. > > 6. It is a nice idea to have an automatically generated secret to avoid > > yet another config. But I wasn't entirely sure, so went for an explicit > > config instead (a bunch of them actually). I had two concerns (a) we > might > > have a password (like the delegation token master secret) that we want > > to encrypt in future that is stored as a cluster-wide password. It will > be > > better if we can configure the broker secret for that, even though for > > that case we will have the same restriction that all brokers are > configured > > with the same secret. (b) broker writes to meta.properties, so there is a > > possibility of losing the secret. > > > > > > On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson > > wrote: > > > > > Hey Rajini, > > > > > > Thanks, makes sense. A couple replies: > > > > > > 2. I haven't changed the way Configurable is used. It is still used for > > > > initial configuration (if the class implements it). Reconfigurable is > > > used > > > > at the moment only for reconfiguration. The reason I did it that way > is > > > > because for some of the internal components, the reconfiguration is > > > handled > > > > separately from initial configuration (we reconfigure classes which > > don't > > > > implement Configurable). But if that is confusing, I can make > > > > Reconfigurable > > > > extend Configurable and add a dummy method in internal classes. What > do > > > you > > > > think? > > > > > > > > > I guess the slight mismatch comes from the difference in initialization > > > between plugins and internal classes. For plugins, we only initialize > > state > > > through configure() so it would be a little weird to have one which was > > > Reconfigurable but not Configurable. Internal classes, on the other > hand, > > > probably have constructors which take the config values explicitly. If > it > > > worked analogously for reconfiguration, I would expect that the > > > reconfigurable internal classes would have an explicit method and would > > not > > > need Reconfigurable at all. That gives us a slightly nicer API for > > testing. > > > That said, if the Reconfigurable API simplifies the internal usage > quite > > a > > > bit, then I have no complaint. > > > > > > 6. I hope not :-) We wouldn't want to store master secret in > ZooKeeper. I > > > > wasn't planning to add encryption for passwords in ZooKeeper > initially > > > and > > > > I think that is ok for keystore passwords. But having started to > > > implement > > > > new listeners which require sasl.jaas.config, I don't think we can > > > release > > > > that with unencrypted passwords in ZooKeeper. We don't really need a > > > master > > > > secret that is same across all brokers since all the password configs > > at > > > > the moment are per-broker configs. So I think I will add a new static > > > > config to the KIP. > > > > > > > > > Haha, agreed. If the configs are pre-broker, you might also consider > > > generating a secret automatically (e.g. it could be added to >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Rajini, 4. Changed is_default to config_source in config_entry in the protocol. It > will be less confusing that way. The method isDefault() will just > return configSource > Would we still include the active config in the list of synonyms? 6. It is a nice idea to have an automatically generated secret to avoid > yet another config. But I wasn't entirely sure, so went for an explicit > config instead (a bunch of them actually). I had two concerns (a) we might > have a password (like the delegation token master secret) that we want > to encrypt in future that is stored as a cluster-wide password. It will be > better if we can configure the broker secret for that, even though for > that case we will have the same restriction that all brokers are configured > with the same secret. (b) broker writes to meta.properties, so there is a > possibility of losing the secret. That's fair. I saw it as similar to auto-generation of the broker-id (if you lose meta.properties, you lose the id also), but maybe it's better to require an explicit config. If users don't provide a config secret, would we store passwords unencrypted or would we forbid them from being altered? Thanks, Jason On Tue, Dec 19, 2017 at 4:16 AM, Rajini Sivaram wrote: > Hi Jason, > > Thank you! > > 2. Updated the KIP: Reconfigurable extends Configurable > 4. Changed is_default to config_source in config_entry in the protocol. It > will be less confusing that way. The method isDefault() will just > return configSource > == DEFAULT_CONFIG. Have also included the changes to the public classes in > the KIP. > 6. It is a nice idea to have an automatically generated secret to avoid > yet another config. But I wasn't entirely sure, so went for an explicit > config instead (a bunch of them actually). I had two concerns (a) we might > have a password (like the delegation token master secret) that we want > to encrypt in future that is stored as a cluster-wide password. It will be > better if we can configure the broker secret for that, even though for > that case we will have the same restriction that all brokers are configured > with the same secret. (b) broker writes to meta.properties, so there is a > possibility of losing the secret. > > > On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson > wrote: > > > Hey Rajini, > > > > Thanks, makes sense. A couple replies: > > > > 2. I haven't changed the way Configurable is used. It is still used for > > > initial configuration (if the class implements it). Reconfigurable is > > used > > > at the moment only for reconfiguration. The reason I did it that way is > > > because for some of the internal components, the reconfiguration is > > handled > > > separately from initial configuration (we reconfigure classes which > don't > > > implement Configurable). But if that is confusing, I can make > > > Reconfigurable > > > extend Configurable and add a dummy method in internal classes. What do > > you > > > think? > > > > > > I guess the slight mismatch comes from the difference in initialization > > between plugins and internal classes. For plugins, we only initialize > state > > through configure() so it would be a little weird to have one which was > > Reconfigurable but not Configurable. Internal classes, on the other hand, > > probably have constructors which take the config values explicitly. If it > > worked analogously for reconfiguration, I would expect that the > > reconfigurable internal classes would have an explicit method and would > not > > need Reconfigurable at all. That gives us a slightly nicer API for > testing. > > That said, if the Reconfigurable API simplifies the internal usage quite > a > > bit, then I have no complaint. > > > > 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I > > > wasn't planning to add encryption for passwords in ZooKeeper initially > > and > > > I think that is ok for keystore passwords. But having started to > > implement > > > new listeners which require sasl.jaas.config, I don't think we can > > release > > > that with unencrypted passwords in ZooKeeper. We don't really need a > > master > > > secret that is same across all brokers since all the password configs > at > > > the moment are per-broker configs. So I think I will add a new static > > > config to the KIP. > > > > > > Haha, agreed. If the configs are pre-broker, you might also consider > > generating a secret automatically (e.g. it could be added to > > meta.properties?). > > > > Thanks, > > Jason > > > > On Mon, Dec 18, 2017 at 4:07 PM, Rajini Sivaram > > > wrote: > > > > > Hi Jason, > > > > > > Thank you for reviewing the KIP. > > > > > > 1. ConfigDef is used for validating the type of the value and the > > > constraints. But I am doing a lot more validation of security configs. > > For > > > example, for keystore configuration update, validate() loads the > keystore > > > and if it is an inter-broker listener, it validates the certificate > chain > > > using the trus
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Jason, Thank you! 2. Updated the KIP: Reconfigurable extends Configurable 4. Changed is_default to config_source in config_entry in the protocol. It will be less confusing that way. The method isDefault() will just return configSource == DEFAULT_CONFIG. Have also included the changes to the public classes in the KIP. 6. It is a nice idea to have an automatically generated secret to avoid yet another config. But I wasn't entirely sure, so went for an explicit config instead (a bunch of them actually). I had two concerns (a) we might have a password (like the delegation token master secret) that we want to encrypt in future that is stored as a cluster-wide password. It will be better if we can configure the broker secret for that, even though for that case we will have the same restriction that all brokers are configured with the same secret. (b) broker writes to meta.properties, so there is a possibility of losing the secret. On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson wrote: > Hey Rajini, > > Thanks, makes sense. A couple replies: > > 2. I haven't changed the way Configurable is used. It is still used for > > initial configuration (if the class implements it). Reconfigurable is > used > > at the moment only for reconfiguration. The reason I did it that way is > > because for some of the internal components, the reconfiguration is > handled > > separately from initial configuration (we reconfigure classes which don't > > implement Configurable). But if that is confusing, I can make > > Reconfigurable > > extend Configurable and add a dummy method in internal classes. What do > you > > think? > > > I guess the slight mismatch comes from the difference in initialization > between plugins and internal classes. For plugins, we only initialize state > through configure() so it would be a little weird to have one which was > Reconfigurable but not Configurable. Internal classes, on the other hand, > probably have constructors which take the config values explicitly. If it > worked analogously for reconfiguration, I would expect that the > reconfigurable internal classes would have an explicit method and would not > need Reconfigurable at all. That gives us a slightly nicer API for testing. > That said, if the Reconfigurable API simplifies the internal usage quite a > bit, then I have no complaint. > > 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I > > wasn't planning to add encryption for passwords in ZooKeeper initially > and > > I think that is ok for keystore passwords. But having started to > implement > > new listeners which require sasl.jaas.config, I don't think we can > release > > that with unencrypted passwords in ZooKeeper. We don't really need a > master > > secret that is same across all brokers since all the password configs at > > the moment are per-broker configs. So I think I will add a new static > > config to the KIP. > > > Haha, agreed. If the configs are pre-broker, you might also consider > generating a secret automatically (e.g. it could be added to > meta.properties?). > > Thanks, > Jason > > On Mon, Dec 18, 2017 at 4:07 PM, Rajini Sivaram > wrote: > > > Hi Jason, > > > > Thank you for reviewing the KIP. > > > > 1. ConfigDef is used for validating the type of the value and the > > constraints. But I am doing a lot more validation of security configs. > For > > example, for keystore configuration update, validate() loads the keystore > > and if it is an inter-broker listener, it validates the certificate chain > > using the truststore for the listener. In the initial configuration case, > > these errors would be detected when the server is started and the server > > would just exit. For dynamic configuration, we want to validate as much > as > > possible before updating the config in ZooKeeper. > > > > 2. I haven't changed the way Configurable is used. It is still used for > > initial configuration (if the class implements it). Reconfigurable is > used > > at the moment only for reconfiguration. The reason I did it that way is > > because for some of the internal components, the reconfiguration is > handled > > separately from initial configuration (we reconfigure classes which don't > > implement Configurable). But if that is confusing, I can make > > Reconfigurable > > extend Configurable and add a dummy method in internal classes. What do > you > > think? > > > > 3. At the moment, I am returning an empty list. Will add the classes to > the > > KIP. > > > > 4. I didn't want to change the existing API, so left the config entry as > > is. When describing synonyms, the entry being described also included in > > the synonym list with its config source. > > > > 5. Configs are validated in groups. validate(Map configs) > > and reconfigure(Map > ?> configs) both provide all the configs (including those not being > > altered). The validator for security configs validates the configs of a > > listener. Validation is performed for altered configs in the
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hey Rajini, Thanks, makes sense. A couple replies: 2. I haven't changed the way Configurable is used. It is still used for > initial configuration (if the class implements it). Reconfigurable is used > at the moment only for reconfiguration. The reason I did it that way is > because for some of the internal components, the reconfiguration is handled > separately from initial configuration (we reconfigure classes which don't > implement Configurable). But if that is confusing, I can make > Reconfigurable > extend Configurable and add a dummy method in internal classes. What do you > think? I guess the slight mismatch comes from the difference in initialization between plugins and internal classes. For plugins, we only initialize state through configure() so it would be a little weird to have one which was Reconfigurable but not Configurable. Internal classes, on the other hand, probably have constructors which take the config values explicitly. If it worked analogously for reconfiguration, I would expect that the reconfigurable internal classes would have an explicit method and would not need Reconfigurable at all. That gives us a slightly nicer API for testing. That said, if the Reconfigurable API simplifies the internal usage quite a bit, then I have no complaint. 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I > wasn't planning to add encryption for passwords in ZooKeeper initially and > I think that is ok for keystore passwords. But having started to implement > new listeners which require sasl.jaas.config, I don't think we can release > that with unencrypted passwords in ZooKeeper. We don't really need a master > secret that is same across all brokers since all the password configs at > the moment are per-broker configs. So I think I will add a new static > config to the KIP. Haha, agreed. If the configs are pre-broker, you might also consider generating a secret automatically (e.g. it could be added to meta.properties?). Thanks, Jason On Mon, Dec 18, 2017 at 4:07 PM, Rajini Sivaram wrote: > Hi Jason, > > Thank you for reviewing the KIP. > > 1. ConfigDef is used for validating the type of the value and the > constraints. But I am doing a lot more validation of security configs. For > example, for keystore configuration update, validate() loads the keystore > and if it is an inter-broker listener, it validates the certificate chain > using the truststore for the listener. In the initial configuration case, > these errors would be detected when the server is started and the server > would just exit. For dynamic configuration, we want to validate as much as > possible before updating the config in ZooKeeper. > > 2. I haven't changed the way Configurable is used. It is still used for > initial configuration (if the class implements it). Reconfigurable is used > at the moment only for reconfiguration. The reason I did it that way is > because for some of the internal components, the reconfiguration is handled > separately from initial configuration (we reconfigure classes which don't > implement Configurable). But if that is confusing, I can make > Reconfigurable > extend Configurable and add a dummy method in internal classes. What do you > think? > > 3. At the moment, I am returning an empty list. Will add the classes to the > KIP. > > 4. I didn't want to change the existing API, so left the config entry as > is. When describing synonyms, the entry being described also included in > the synonym list with its config source. > > 5. Configs are validated in groups. validate(Map configs) > and reconfigure(Map ?> configs) both provide all the configs (including those not being > altered). The validator for security configs validates the configs of a > listener. Validation is performed for altered configs in the context of > the complete new config. The ordering in AlterConfigs is not important, > validation is performed on the map. > > 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I > wasn't planning to add encryption for passwords in ZooKeeper initially and > I think that is ok for keystore passwords. But having started to implement > new listeners which require sasl.jaas.config, I don't think we can release > that with unencrypted passwords in ZooKeeper. We don't really need a master > secret that is same across all brokers since all the password configs at > the moment are per-broker configs. So I think I will add a new static > config to the KIP. > > > > On Mon, Dec 18, 2017 at 9:40 PM, Jason Gustafson > wrote: > > > Hi Rajini, > > > > Looking good. Just a few questions. > > > > 1. (Related to Jay's comment) Is the validate() method on Reconfigurable > > necessary? I would have thought we'd validate using the ConfigDef. Do you > > have a use case in mind in which the reconfigurable component only > permits > > certain reconfigurations? > > 2. Should Reconfigurable extend Configurable or is the initial > > configuration also done through reconf
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Jason, Thank you for reviewing the KIP. 1. ConfigDef is used for validating the type of the value and the constraints. But I am doing a lot more validation of security configs. For example, for keystore configuration update, validate() loads the keystore and if it is an inter-broker listener, it validates the certificate chain using the truststore for the listener. In the initial configuration case, these errors would be detected when the server is started and the server would just exit. For dynamic configuration, we want to validate as much as possible before updating the config in ZooKeeper. 2. I haven't changed the way Configurable is used. It is still used for initial configuration (if the class implements it). Reconfigurable is used at the moment only for reconfiguration. The reason I did it that way is because for some of the internal components, the reconfiguration is handled separately from initial configuration (we reconfigure classes which don't implement Configurable). But if that is confusing, I can make Reconfigurable extend Configurable and add a dummy method in internal classes. What do you think? 3. At the moment, I am returning an empty list. Will add the classes to the KIP. 4. I didn't want to change the existing API, so left the config entry as is. When describing synonyms, the entry being described also included in the synonym list with its config source. 5. Configs are validated in groups. validate(Map configs) and reconfigure(Map configs) both provide all the configs (including those not being altered). The validator for security configs validates the configs of a listener. Validation is performed for altered configs in the context of the complete new config. The ordering in AlterConfigs is not important, validation is performed on the map. 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I wasn't planning to add encryption for passwords in ZooKeeper initially and I think that is ok for keystore passwords. But having started to implement new listeners which require sasl.jaas.config, I don't think we can release that with unencrypted passwords in ZooKeeper. We don't really need a master secret that is same across all brokers since all the password configs at the moment are per-broker configs. So I think I will add a new static config to the KIP. On Mon, Dec 18, 2017 at 9:40 PM, Jason Gustafson wrote: > Hi Rajini, > > Looking good. Just a few questions. > > 1. (Related to Jay's comment) Is the validate() method on Reconfigurable > necessary? I would have thought we'd validate using the ConfigDef. Do you > have a use case in mind in which the reconfigurable component only permits > certain reconfigurations? > 2. Should Reconfigurable extend Configurable or is the initial > configuration also done through reconfigure()? I ask because not all > plugins interfaces currently extend Configurable (e.g. > KafkaPrincipalBuilder). > 3. You mentioned a couple changes to DescribeConfigsOptions and > DescribeConfigsResult. Perhaps we should list the changes explicitly? One > not totally obvious case is what the synonyms() getter would return if the > option is not specified (i.e. should it raise an exception or return an > empty list?). > 4. Config entries in the DescribeConfigs response have an is_default flag. > Could that be replaced with the more general config_source? > 5. Bit of an internal question, but how do you handle config dependencies? > For example, suppose I want to add a listener and configure its principal > builder at once. You'd have to validate the principal builder config in the > context of the listener config, so I guess the order of the entries in > AlterConfigs is significant? > 6. KIP-48 (delegation tokens) gives us a master secret which is shared by > all brokers. Do you think we would make this dynamically configurable? > Alternatively, it might be possible to use it to encrypt the other > passwords we store in zookeeper. > > Thanks, > Jason > > > > On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram > wrote: > > > Hi Jay, > > > > Thank you for reviewing the KIP. > > > > 1) Yes, makes sense. I will update the PR. There are some config updates > > that may be allowed depending on the context (e.g. some security configs > > can be updated for new listeners, but not existing listeners). Perhaps it > > is ok to mark them dynamic in the documentation. AdminClient would give > > appropriate error messages if the update is not allowed. > > 2) Internally, in the implementation, a mixture of direct config updates > > (e.g log config as you have pointed out) and reconfigure method > invocations > > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > > reporter), we require the Reconfigurable interface to ensure that we can > > validate any custom configs and avoid reconfiguration for plugin versions > > that don't support it. > > > > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > > > > > Two thoughts on implementation (
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Rajini, Looking good. Just a few questions. 1. (Related to Jay's comment) Is the validate() method on Reconfigurable necessary? I would have thought we'd validate using the ConfigDef. Do you have a use case in mind in which the reconfigurable component only permits certain reconfigurations? 2. Should Reconfigurable extend Configurable or is the initial configuration also done through reconfigure()? I ask because not all plugins interfaces currently extend Configurable (e.g. KafkaPrincipalBuilder). 3. You mentioned a couple changes to DescribeConfigsOptions and DescribeConfigsResult. Perhaps we should list the changes explicitly? One not totally obvious case is what the synonyms() getter would return if the option is not specified (i.e. should it raise an exception or return an empty list?). 4. Config entries in the DescribeConfigs response have an is_default flag. Could that be replaced with the more general config_source? 5. Bit of an internal question, but how do you handle config dependencies? For example, suppose I want to add a listener and configure its principal builder at once. You'd have to validate the principal builder config in the context of the listener config, so I guess the order of the entries in AlterConfigs is significant? 6. KIP-48 (delegation tokens) gives us a master secret which is shared by all brokers. Do you think we would make this dynamically configurable? Alternatively, it might be possible to use it to encrypt the other passwords we store in zookeeper. Thanks, Jason On Mon, Dec 18, 2017 at 10:16 AM, Rajini Sivaram wrote: > Hi Jay, > > Thank you for reviewing the KIP. > > 1) Yes, makes sense. I will update the PR. There are some config updates > that may be allowed depending on the context (e.g. some security configs > can be updated for new listeners, but not existing listeners). Perhaps it > is ok to mark them dynamic in the documentation. AdminClient would give > appropriate error messages if the update is not allowed. > 2) Internally, in the implementation, a mixture of direct config updates > (e.g log config as you have pointed out) and reconfigure method invocations > (e.g. SslFactory) are used. For configurable plugins (e.g. metrics > reporter), we require the Reconfigurable interface to ensure that we can > validate any custom configs and avoid reconfiguration for plugin versions > that don't support it. > > > On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > > > Two thoughts on implementation (shouldn't effect the KIP): > > > >1. It might be nice to add a parameter to ConfigDef which says > whether a > >configuration is dynamically updatable or not so that we can give > error > >messages if it isn't and also have it reflected in the auto-generated > > docs. > >2. For many systems they don't really need to take action if a config > >changes, they just need to use the new value. Changing them all to > >Reconfigurable requires managing a fair amount of mutability in each > > class > >that accepts changes. Some need this since they need to take actions > > when a > >config changes, but it seems like many just need to update some value. > > For > >the later you might just be able to do something like what we do for > >LogConfig where there is a single CurrentConfig instance that has a > >reference to the current KafkaConfig and always reference your > > configurable > >parameters via this (e.g. config.current.myConfig). Dunno if that is > >actually better, but thought I'd throw it out there. > > > > -Jay > > > > On Sun, Dec 10, 2017 at 8:09 AM, Rajini Sivaram > > > wrote: > > > > > Hi Jun, > > > > > > Thank you! > > > > > > 5. Yes, that makes sense. Agree that we don't want to add protocol > > changes > > > to *UpdateMetadataRequest* in this KIP. I have moved the update of > > > *log.message.format.version* and *inter.broker.protocol.version* to > > reduce > > > restarts during upgrade to* Future Work*. We can do this in a follow-on > > > KIP. > > > > > > I will wait for another day to see if there are any more comments and > > start > > > vote on Tuesday if there are no other concerns. > > > > > > > > > On Sat, Dec 9, 2017 at 12:22 AM, Jun Rao wrote: > > > > > > > Hi, Rajini, > > > > > > > > Thanks for the reply. They all make sense. > > > > > > > > 5. Got it. Note that currently, only live brokers are registered in > ZK. > > > > Another thing is that I am not sure that we want every broker to read > > all > > > > broker registrations directly from ZK. It's probably better to have > the > > > > controller propagate this information. That will require changing the > > > > UpdateMetadataRequest protocol though. So, I am not sure if you want > to > > > do > > > > that in the same KIP. > > > > > > > > Jun > > > > > > > > > > > > > > > > On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Jun, > > > > > > > > > > Thank you for the review. > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Jay, Thank you for reviewing the KIP. 1) Yes, makes sense. I will update the PR. There are some config updates that may be allowed depending on the context (e.g. some security configs can be updated for new listeners, but not existing listeners). Perhaps it is ok to mark them dynamic in the documentation. AdminClient would give appropriate error messages if the update is not allowed. 2) Internally, in the implementation, a mixture of direct config updates (e.g log config as you have pointed out) and reconfigure method invocations (e.g. SslFactory) are used. For configurable plugins (e.g. metrics reporter), we require the Reconfigurable interface to ensure that we can validate any custom configs and avoid reconfiguration for plugin versions that don't support it. On Mon, Dec 18, 2017 at 5:49 PM, Jay Kreps wrote: > Two thoughts on implementation (shouldn't effect the KIP): > >1. It might be nice to add a parameter to ConfigDef which says whether a >configuration is dynamically updatable or not so that we can give error >messages if it isn't and also have it reflected in the auto-generated > docs. >2. For many systems they don't really need to take action if a config >changes, they just need to use the new value. Changing them all to >Reconfigurable requires managing a fair amount of mutability in each > class >that accepts changes. Some need this since they need to take actions > when a >config changes, but it seems like many just need to update some value. > For >the later you might just be able to do something like what we do for >LogConfig where there is a single CurrentConfig instance that has a >reference to the current KafkaConfig and always reference your > configurable >parameters via this (e.g. config.current.myConfig). Dunno if that is >actually better, but thought I'd throw it out there. > > -Jay > > On Sun, Dec 10, 2017 at 8:09 AM, Rajini Sivaram > wrote: > > > Hi Jun, > > > > Thank you! > > > > 5. Yes, that makes sense. Agree that we don't want to add protocol > changes > > to *UpdateMetadataRequest* in this KIP. I have moved the update of > > *log.message.format.version* and *inter.broker.protocol.version* to > reduce > > restarts during upgrade to* Future Work*. We can do this in a follow-on > > KIP. > > > > I will wait for another day to see if there are any more comments and > start > > vote on Tuesday if there are no other concerns. > > > > > > On Sat, Dec 9, 2017 at 12:22 AM, Jun Rao wrote: > > > > > Hi, Rajini, > > > > > > Thanks for the reply. They all make sense. > > > > > > 5. Got it. Note that currently, only live brokers are registered in ZK. > > > Another thing is that I am not sure that we want every broker to read > all > > > broker registrations directly from ZK. It's probably better to have the > > > controller propagate this information. That will require changing the > > > UpdateMetadataRequest protocol though. So, I am not sure if you want to > > do > > > that in the same KIP. > > > > > > Jun > > > > > > > > > > > > On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram < > rajinisiva...@gmail.com> > > > wrote: > > > > > > > Hi Jun, > > > > > > > > Thank you for the review. > > > > > > > > 1. No, I am hoping to migrate partitions to new threads. We just need > > to > > > > ensure they don't run concurrently. > > > > > > > > 2. AdminClient has a validateOnly option for AlterConfigs. Any > > exceptions > > > > or return value of false from Reconfigurable#validate would fail the > > > > AlterConfigsRequest. > > > > > > > > 3. Yes, we will support describe and alter of configs with listener > > > prefix. > > > > We will not allow alterConfigs() of security configs without the > > listener > > > > prefix, since we need to prevent the whole cluster being made > unusable. > > > > > > > > 4. Thank you, will make a note of that. > > > > > > > > 5. When we are upgrading (from 1.0 to 2.0 for example), my > > understanding > > > is > > > > that we set inter.broker.protocol.version=1.0, do a rolling upgrade > of > > > the > > > > whole cluster and when all brokers are at 2.0, we do another rolling > > > > upgrade with inter.broker.protocol.version=2.0. Jason's suggestion > was > > > to > > > > avoid the second rolling upgrade by enabling dynamic update of > > > > inter.broker.protocol.version. To set inter.broker.protocol.version= > > 2.0 > > > > dynamically, we need to verify not just that the current broker is on > > > > version 2.0, but that all brokers int the cluster are on version 2.0 > (I > > > > thought that was the reason for the second rolling upgrade). The > broker > > > > version in ZK would enable that verification before performing the > > > update. > > > > > > > > 6. The config source would be STATIC_BROKER_CONFIG/DYNAMIC_ > > > BROKER_CONFIG, > > > > the config name would be listener.name.listenerA.configX. And > synonyms > > > > list > > > > in describeConfigs() would list listener.name.listenerA.configX as > > well
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Two thoughts on implementation (shouldn't effect the KIP): 1. It might be nice to add a parameter to ConfigDef which says whether a configuration is dynamically updatable or not so that we can give error messages if it isn't and also have it reflected in the auto-generated docs. 2. For many systems they don't really need to take action if a config changes, they just need to use the new value. Changing them all to Reconfigurable requires managing a fair amount of mutability in each class that accepts changes. Some need this since they need to take actions when a config changes, but it seems like many just need to update some value. For the later you might just be able to do something like what we do for LogConfig where there is a single CurrentConfig instance that has a reference to the current KafkaConfig and always reference your configurable parameters via this (e.g. config.current.myConfig). Dunno if that is actually better, but thought I'd throw it out there. -Jay On Sun, Dec 10, 2017 at 8:09 AM, Rajini Sivaram wrote: > Hi Jun, > > Thank you! > > 5. Yes, that makes sense. Agree that we don't want to add protocol changes > to *UpdateMetadataRequest* in this KIP. I have moved the update of > *log.message.format.version* and *inter.broker.protocol.version* to reduce > restarts during upgrade to* Future Work*. We can do this in a follow-on > KIP. > > I will wait for another day to see if there are any more comments and start > vote on Tuesday if there are no other concerns. > > > On Sat, Dec 9, 2017 at 12:22 AM, Jun Rao wrote: > > > Hi, Rajini, > > > > Thanks for the reply. They all make sense. > > > > 5. Got it. Note that currently, only live brokers are registered in ZK. > > Another thing is that I am not sure that we want every broker to read all > > broker registrations directly from ZK. It's probably better to have the > > controller propagate this information. That will require changing the > > UpdateMetadataRequest protocol though. So, I am not sure if you want to > do > > that in the same KIP. > > > > Jun > > > > > > > > On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram > > wrote: > > > > > Hi Jun, > > > > > > Thank you for the review. > > > > > > 1. No, I am hoping to migrate partitions to new threads. We just need > to > > > ensure they don't run concurrently. > > > > > > 2. AdminClient has a validateOnly option for AlterConfigs. Any > exceptions > > > or return value of false from Reconfigurable#validate would fail the > > > AlterConfigsRequest. > > > > > > 3. Yes, we will support describe and alter of configs with listener > > prefix. > > > We will not allow alterConfigs() of security configs without the > listener > > > prefix, since we need to prevent the whole cluster being made unusable. > > > > > > 4. Thank you, will make a note of that. > > > > > > 5. When we are upgrading (from 1.0 to 2.0 for example), my > understanding > > is > > > that we set inter.broker.protocol.version=1.0, do a rolling upgrade of > > the > > > whole cluster and when all brokers are at 2.0, we do another rolling > > > upgrade with inter.broker.protocol.version=2.0. Jason's suggestion was > > to > > > avoid the second rolling upgrade by enabling dynamic update of > > > inter.broker.protocol.version. To set inter.broker.protocol.version= > 2.0 > > > dynamically, we need to verify not just that the current broker is on > > > version 2.0, but that all brokers int the cluster are on version 2.0 (I > > > thought that was the reason for the second rolling upgrade). The broker > > > version in ZK would enable that verification before performing the > > update. > > > > > > 6. The config source would be STATIC_BROKER_CONFIG/DYNAMIC_ > > BROKER_CONFIG, > > > the config name would be listener.name.listenerA.configX. And synonyms > > > list > > > in describeConfigs() would list listener.name.listenerA.configX as > well > > > as > > > configX. > > > > > > 7. I think `default` is an overused terminology already. When configs > are > > > described, they return a flag indicating if the value is a default. And > > in > > > the broker, we have so many levels of configs already and we are adding > > so > > > many more, that it may be better to use a different term. It doesn't > have > > > to be synonyms, but since we want to use the same term for topics and > > > brokers and we have listener configs which override non-prefixed > security > > > configs, perhaps it is ok? > > > > > > Regards, > > > > > > Rajini > > > > > > > > > > > > On Wed, Dec 6, 2017 at 11:50 PM, Jun Rao wrote: > > > > > > > A couple more things. > > > > > > > > 6. For the SSL/SASL configurations with the listener prefix, do we > need > > > > another level in config_source since it's neither topic nor broker? > > > > > > > > 7. For include_synonyms in DescribeConfigs, the name makes sense for > > the > > > > topic level configs. Not sure if it makes sense for other > hierarchies. > > > > Perhaps sth more generic like defa
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Jun, Thank you! 5. Yes, that makes sense. Agree that we don't want to add protocol changes to *UpdateMetadataRequest* in this KIP. I have moved the update of *log.message.format.version* and *inter.broker.protocol.version* to reduce restarts during upgrade to* Future Work*. We can do this in a follow-on KIP. I will wait for another day to see if there are any more comments and start vote on Tuesday if there are no other concerns. On Sat, Dec 9, 2017 at 12:22 AM, Jun Rao wrote: > Hi, Rajini, > > Thanks for the reply. They all make sense. > > 5. Got it. Note that currently, only live brokers are registered in ZK. > Another thing is that I am not sure that we want every broker to read all > broker registrations directly from ZK. It's probably better to have the > controller propagate this information. That will require changing the > UpdateMetadataRequest protocol though. So, I am not sure if you want to do > that in the same KIP. > > Jun > > > > On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram > wrote: > > > Hi Jun, > > > > Thank you for the review. > > > > 1. No, I am hoping to migrate partitions to new threads. We just need to > > ensure they don't run concurrently. > > > > 2. AdminClient has a validateOnly option for AlterConfigs. Any exceptions > > or return value of false from Reconfigurable#validate would fail the > > AlterConfigsRequest. > > > > 3. Yes, we will support describe and alter of configs with listener > prefix. > > We will not allow alterConfigs() of security configs without the listener > > prefix, since we need to prevent the whole cluster being made unusable. > > > > 4. Thank you, will make a note of that. > > > > 5. When we are upgrading (from 1.0 to 2.0 for example), my understanding > is > > that we set inter.broker.protocol.version=1.0, do a rolling upgrade of > the > > whole cluster and when all brokers are at 2.0, we do another rolling > > upgrade with inter.broker.protocol.version=2.0. Jason's suggestion was > to > > avoid the second rolling upgrade by enabling dynamic update of > > inter.broker.protocol.version. To set inter.broker.protocol.version=2.0 > > dynamically, we need to verify not just that the current broker is on > > version 2.0, but that all brokers int the cluster are on version 2.0 (I > > thought that was the reason for the second rolling upgrade). The broker > > version in ZK would enable that verification before performing the > update. > > > > 6. The config source would be STATIC_BROKER_CONFIG/DYNAMIC_ > BROKER_CONFIG, > > the config name would be listener.name.listenerA.configX. And synonyms > > list > > in describeConfigs() would list listener.name.listenerA.configX as well > > as > > configX. > > > > 7. I think `default` is an overused terminology already. When configs are > > described, they return a flag indicating if the value is a default. And > in > > the broker, we have so many levels of configs already and we are adding > so > > many more, that it may be better to use a different term. It doesn't have > > to be synonyms, but since we want to use the same term for topics and > > brokers and we have listener configs which override non-prefixed security > > configs, perhaps it is ok? > > > > Regards, > > > > Rajini > > > > > > > > On Wed, Dec 6, 2017 at 11:50 PM, Jun Rao wrote: > > > > > A couple more things. > > > > > > 6. For the SSL/SASL configurations with the listener prefix, do we need > > > another level in config_source since it's neither topic nor broker? > > > > > > 7. For include_synonyms in DescribeConfigs, the name makes sense for > the > > > topic level configs. Not sure if it makes sense for other hierarchies. > > > Perhaps sth more generic like default will be better? > > > > > > Thanks, > > > > > > Jun > > > > > > On Wed, Dec 6, 2017 at 3:41 PM, Jun Rao wrote: > > > > > > > Hi, Rajini, > > > > > > > > Thanks for the kip. Looks good overall. A few comments below. > > > > > > > > 1. "num.replica.fetchers: Affinity of partitions to threads will be > > > > preserved for ordering." Does that mean the new fetcher threads won't > > be > > > > used until new partitions are added? This may be limiting. > > > > > > > > 2. I am wondering how the result from reporter.validate(Map ?> > > > > configs) will be used. If it returns false, do we return false to the > > > admin > > > > client for the validation call, which will seem a bit weird? > > > > > > > > 3. For the SSL and SASL configuration changes, do we support those > with > > > > the listener prefix (e.g., external-ssl-lisener.ssl. > > keystore.location). > > > > If so, do we plan to include them in the result of describeConfigs()? > > > > > > > > 4. "Updates to advertised.listeners will re-register the new listener > > in > > > > ZK". To support this, we will likely need additional logic in the > > > > controller such that the controller can broadcast the metadata with > the > > > new > > > > listeners to every broker. > > > > > > > > 5. Including broker version in broker registrati
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi, Rajini, Thanks for the reply. They all make sense. 5. Got it. Note that currently, only live brokers are registered in ZK. Another thing is that I am not sure that we want every broker to read all broker registrations directly from ZK. It's probably better to have the controller propagate this information. That will require changing the UpdateMetadataRequest protocol though. So, I am not sure if you want to do that in the same KIP. Jun On Fri, Dec 8, 2017 at 6:07 AM, Rajini Sivaram wrote: > Hi Jun, > > Thank you for the review. > > 1. No, I am hoping to migrate partitions to new threads. We just need to > ensure they don't run concurrently. > > 2. AdminClient has a validateOnly option for AlterConfigs. Any exceptions > or return value of false from Reconfigurable#validate would fail the > AlterConfigsRequest. > > 3. Yes, we will support describe and alter of configs with listener prefix. > We will not allow alterConfigs() of security configs without the listener > prefix, since we need to prevent the whole cluster being made unusable. > > 4. Thank you, will make a note of that. > > 5. When we are upgrading (from 1.0 to 2.0 for example), my understanding is > that we set inter.broker.protocol.version=1.0, do a rolling upgrade of the > whole cluster and when all brokers are at 2.0, we do another rolling > upgrade with inter.broker.protocol.version=2.0. Jason's suggestion was to > avoid the second rolling upgrade by enabling dynamic update of > inter.broker.protocol.version. To set inter.broker.protocol.version=2.0 > dynamically, we need to verify not just that the current broker is on > version 2.0, but that all brokers int the cluster are on version 2.0 (I > thought that was the reason for the second rolling upgrade). The broker > version in ZK would enable that verification before performing the update. > > 6. The config source would be STATIC_BROKER_CONFIG/DYNAMIC_BROKER_CONFIG, > the config name would be listener.name.listenerA.configX. And synonyms > list > in describeConfigs() would list listener.name.listenerA.configX as well > as > configX. > > 7. I think `default` is an overused terminology already. When configs are > described, they return a flag indicating if the value is a default. And in > the broker, we have so many levels of configs already and we are adding so > many more, that it may be better to use a different term. It doesn't have > to be synonyms, but since we want to use the same term for topics and > brokers and we have listener configs which override non-prefixed security > configs, perhaps it is ok? > > Regards, > > Rajini > > > > On Wed, Dec 6, 2017 at 11:50 PM, Jun Rao wrote: > > > A couple more things. > > > > 6. For the SSL/SASL configurations with the listener prefix, do we need > > another level in config_source since it's neither topic nor broker? > > > > 7. For include_synonyms in DescribeConfigs, the name makes sense for the > > topic level configs. Not sure if it makes sense for other hierarchies. > > Perhaps sth more generic like default will be better? > > > > Thanks, > > > > Jun > > > > On Wed, Dec 6, 2017 at 3:41 PM, Jun Rao wrote: > > > > > Hi, Rajini, > > > > > > Thanks for the kip. Looks good overall. A few comments below. > > > > > > 1. "num.replica.fetchers: Affinity of partitions to threads will be > > > preserved for ordering." Does that mean the new fetcher threads won't > be > > > used until new partitions are added? This may be limiting. > > > > > > 2. I am wondering how the result from reporter.validate(Map > > > configs) will be used. If it returns false, do we return false to the > > admin > > > client for the validation call, which will seem a bit weird? > > > > > > 3. For the SSL and SASL configuration changes, do we support those with > > > the listener prefix (e.g., external-ssl-lisener.ssl. > keystore.location). > > > If so, do we plan to include them in the result of describeConfigs()? > > > > > > 4. "Updates to advertised.listeners will re-register the new listener > in > > > ZK". To support this, we will likely need additional logic in the > > > controller such that the controller can broadcast the metadata with the > > new > > > listeners to every broker. > > > > > > 5. Including broker version in broker registration in ZK. I am not sure > > > the usage of that. Each broker knows its version. So, is that for the > > > controller? > > > > > > Jun > > > > > > > > > > > > On Tue, Dec 5, 2017 at 11:05 AM, Colin McCabe > > wrote: > > > > > >> On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram wrote: > > >> > Hi Colin, > > >> > > > >> > KAFKA-5722 already has an owner, so I didn't want to confuse the two > > >> > KIPs. They can be done independently of each other. The goal is to > > try > > >> and > > >> > validate every config to the minimum validation already in the > broker > > >> for > > >> > the static configs, but in some cases to a more restrictive level. > So > > a > > >> > typo like a file-not-found or class-not-found would definitel
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Jun, Thank you for the review. 1. No, I am hoping to migrate partitions to new threads. We just need to ensure they don't run concurrently. 2. AdminClient has a validateOnly option for AlterConfigs. Any exceptions or return value of false from Reconfigurable#validate would fail the AlterConfigsRequest. 3. Yes, we will support describe and alter of configs with listener prefix. We will not allow alterConfigs() of security configs without the listener prefix, since we need to prevent the whole cluster being made unusable. 4. Thank you, will make a note of that. 5. When we are upgrading (from 1.0 to 2.0 for example), my understanding is that we set inter.broker.protocol.version=1.0, do a rolling upgrade of the whole cluster and when all brokers are at 2.0, we do another rolling upgrade with inter.broker.protocol.version=2.0. Jason's suggestion was to avoid the second rolling upgrade by enabling dynamic update of inter.broker.protocol.version. To set inter.broker.protocol.version=2.0 dynamically, we need to verify not just that the current broker is on version 2.0, but that all brokers int the cluster are on version 2.0 (I thought that was the reason for the second rolling upgrade). The broker version in ZK would enable that verification before performing the update. 6. The config source would be STATIC_BROKER_CONFIG/DYNAMIC_BROKER_CONFIG, the config name would be listener.name.listenerA.configX. And synonyms list in describeConfigs() would list listener.name.listenerA.configX as well as configX. 7. I think `default` is an overused terminology already. When configs are described, they return a flag indicating if the value is a default. And in the broker, we have so many levels of configs already and we are adding so many more, that it may be better to use a different term. It doesn't have to be synonyms, but since we want to use the same term for topics and brokers and we have listener configs which override non-prefixed security configs, perhaps it is ok? Regards, Rajini On Wed, Dec 6, 2017 at 11:50 PM, Jun Rao wrote: > A couple more things. > > 6. For the SSL/SASL configurations with the listener prefix, do we need > another level in config_source since it's neither topic nor broker? > > 7. For include_synonyms in DescribeConfigs, the name makes sense for the > topic level configs. Not sure if it makes sense for other hierarchies. > Perhaps sth more generic like default will be better? > > Thanks, > > Jun > > On Wed, Dec 6, 2017 at 3:41 PM, Jun Rao wrote: > > > Hi, Rajini, > > > > Thanks for the kip. Looks good overall. A few comments below. > > > > 1. "num.replica.fetchers: Affinity of partitions to threads will be > > preserved for ordering." Does that mean the new fetcher threads won't be > > used until new partitions are added? This may be limiting. > > > > 2. I am wondering how the result from reporter.validate(Map > > configs) will be used. If it returns false, do we return false to the > admin > > client for the validation call, which will seem a bit weird? > > > > 3. For the SSL and SASL configuration changes, do we support those with > > the listener prefix (e.g., external-ssl-lisener.ssl.keystore.location). > > If so, do we plan to include them in the result of describeConfigs()? > > > > 4. "Updates to advertised.listeners will re-register the new listener in > > ZK". To support this, we will likely need additional logic in the > > controller such that the controller can broadcast the metadata with the > new > > listeners to every broker. > > > > 5. Including broker version in broker registration in ZK. I am not sure > > the usage of that. Each broker knows its version. So, is that for the > > controller? > > > > Jun > > > > > > > > On Tue, Dec 5, 2017 at 11:05 AM, Colin McCabe > wrote: > > > >> On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram wrote: > >> > Hi Colin, > >> > > >> > KAFKA-5722 already has an owner, so I didn't want to confuse the two > >> > KIPs. They can be done independently of each other. The goal is to > try > >> and > >> > validate every config to the minimum validation already in the broker > >> for > >> > the static configs, but in some cases to a more restrictive level. So > a > >> > typo like a file-not-found or class-not-found would definitely fail > the > >> > AlterConfigs request (validation is performed by AlterConfigs > regardless > >> > of validateOnly flag). I am working out the additional validation I > can > >> > perform as I implement updates for each config. For example, > >> > inter-broker keystore update will not be allowed unless it can be > >> > verified against the currently configured truststore. > >> > >> HI Rajini, > >> > >> I agree. It's probably better to avoid expanding the scope of KIP-226. > >> I hope we can get to KAFKA-5722 soon, though, since it will really > >> improve the user experience for this feature. > >> > >> regards, > >> Colin > >> > >> > > >> > On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe > >> wrote: > >> > > >> > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
A couple more things. 6. For the SSL/SASL configurations with the listener prefix, do we need another level in config_source since it's neither topic nor broker? 7. For include_synonyms in DescribeConfigs, the name makes sense for the topic level configs. Not sure if it makes sense for other hierarchies. Perhaps sth more generic like default will be better? Thanks, Jun On Wed, Dec 6, 2017 at 3:41 PM, Jun Rao wrote: > Hi, Rajini, > > Thanks for the kip. Looks good overall. A few comments below. > > 1. "num.replica.fetchers: Affinity of partitions to threads will be > preserved for ordering." Does that mean the new fetcher threads won't be > used until new partitions are added? This may be limiting. > > 2. I am wondering how the result from reporter.validate(Map > configs) will be used. If it returns false, do we return false to the admin > client for the validation call, which will seem a bit weird? > > 3. For the SSL and SASL configuration changes, do we support those with > the listener prefix (e.g., external-ssl-lisener.ssl.keystore.location). > If so, do we plan to include them in the result of describeConfigs()? > > 4. "Updates to advertised.listeners will re-register the new listener in > ZK". To support this, we will likely need additional logic in the > controller such that the controller can broadcast the metadata with the new > listeners to every broker. > > 5. Including broker version in broker registration in ZK. I am not sure > the usage of that. Each broker knows its version. So, is that for the > controller? > > Jun > > > > On Tue, Dec 5, 2017 at 11:05 AM, Colin McCabe wrote: > >> On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram wrote: >> > Hi Colin, >> > >> > KAFKA-5722 already has an owner, so I didn't want to confuse the two >> > KIPs. They can be done independently of each other. The goal is to try >> and >> > validate every config to the minimum validation already in the broker >> for >> > the static configs, but in some cases to a more restrictive level. So a >> > typo like a file-not-found or class-not-found would definitely fail the >> > AlterConfigs request (validation is performed by AlterConfigs regardless >> > of validateOnly flag). I am working out the additional validation I can >> > perform as I implement updates for each config. For example, >> > inter-broker keystore update will not be allowed unless it can be >> > verified against the currently configured truststore. >> >> HI Rajini, >> >> I agree. It's probably better to avoid expanding the scope of KIP-226. >> I hope we can get to KAFKA-5722 soon, though, since it will really >> improve the user experience for this feature. >> >> regards, >> Colin >> >> > >> > On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe >> wrote: >> > >> > > 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 d
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi, Rajini, Thanks for the kip. Looks good overall. A few comments below. 1. "num.replica.fetchers: Affinity of partitions to threads will be preserved for ordering." Does that mean the new fetcher threads won't be used until new partitions are added? This may be limiting. 2. I am wondering how the result from reporter.validate(Map configs) will be used. If it returns false, do we return false to the admin client for the validation call, which will seem a bit weird? 3. For the SSL and SASL configuration changes, do we support those with the listener prefix (e.g., external-ssl-lisener.ssl.keystore.location). If so, do we plan to include them in the result of describeConfigs()? 4. "Updates to advertised.listeners will re-register the new listener in ZK". To support this, we will likely need additional logic in the controller such that the controller can broadcast the metadata with the new listeners to every broker. 5. Including broker version in broker registration in ZK. I am not sure the usage of that. Each broker knows its version. So, is that for the controller? Jun On Tue, Dec 5, 2017 at 11:05 AM, Colin McCabe wrote: > On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram wrote: > > Hi Colin, > > > > KAFKA-5722 already has an owner, so I didn't want to confuse the two > > KIPs. They can be done independently of each other. The goal is to try > and > > validate every config to the minimum validation already in the broker for > > the static configs, but in some cases to a more restrictive level. So a > > typo like a file-not-found or class-not-found would definitely fail the > > AlterConfigs request (validation is performed by AlterConfigs regardless > > of validateOnly flag). I am working out the additional validation I can > > perform as I implement updates for each config. For example, > > inter-broker keystore update will not be allowed unless it can be > > verified against the currently configured truststore. > > HI Rajini, > > I agree. It's probably better to avoid expanding the scope of KIP-226. > I hope we can get to KAFKA-5722 soon, though, since it will really > improve the user experience for this feature. > > regards, > Colin > > > > > On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe > wrote: > > > > > 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 > > > 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 fo
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
On Tue, Dec 5, 2017, at 06:01, Rajini Sivaram wrote: > Hi Colin, > > KAFKA-5722 already has an owner, so I didn't want to confuse the two > KIPs. They can be done independently of each other. The goal is to try and > validate every config to the minimum validation already in the broker for > the static configs, but in some cases to a more restrictive level. So a > typo like a file-not-found or class-not-found would definitely fail the > AlterConfigs request (validation is performed by AlterConfigs regardless > of validateOnly flag). I am working out the additional validation I can > perform as I implement updates for each config. For example, > inter-broker keystore update will not be allowed unless it can be > verified against the currently configured truststore. HI Rajini, I agree. It's probably better to avoid expanding the scope of KIP-226. I hope we can get to KAFKA-5722 soon, though, since it will really improve the user experience for this feature. regards, Colin > > On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe wrote: > > > 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 > > 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
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Colin, KAFKA-5722 already has an owner, so I didn't want to confuse the two KIPs. They can be done independently of each other. The goal is to try and validate every config to the minimum validation already in the broker for the static configs, but in some cases to a more restrictive level. So a typo like a file-not-found or class-not-found would definitely fail the AlterConfigs request (validation is performed by AlterConfigs regardless of validateOnly flag). I am working out the additional validation I can perform as I implement updates for each config. For example, inter-broker keystore update will not be allowed unless it can be verified against the currently configured truststore. On Sat, Dec 2, 2017 at 10:15 PM, Colin McCabe wrote: > 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 > 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 rev
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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, Ra
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Thanks for the explanation. On Fri, Dec 1, 2017 at 10:13 AM, Rajini Sivaram wrote: > Hi Ted, > > Thank you for the review. */config/brokers/id *contains the persistent > configuration of broker *. *That will be configured using > kafka-configs.sh/AdminClient. */brokers/ids/id* contains the ephemeral > metadata registered by broker *.* For broker version, we don't want the > data to outlive the broker. Hence adding it to */brokers/ids/id.* > > On Fri, Dec 1, 2017 at 5:38 PM, Ted Yu wrote: > > > 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 > > 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 > > > >>> de
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi Ted, Thank you for the review. */config/brokers/id *contains the persistent configuration of broker *. *That will be configured using kafka-configs.sh/AdminClient. */brokers/ids/id* contains the ephemeral metadata registered by broker *.* For broker version, we don't want the data to outlive the broker. Hence adding it to */brokers/ids/id.* On Fri, Dec 1, 2017 at 5:38 PM, Ted Yu wrote: > 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 > 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 > > 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 > > > >> 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 > > >>> wrote: > > >>> > > > >>> > > Hi Rajini, > > >>> > > > > >>> > > This seems like a nice improvement! > > >>> > > > > >>> > > One thing that is a bit concernin
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > 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 > >> 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 > >>> 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 t
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > 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 >> 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 >> > >>> 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 >>> 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
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > 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 >> 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 >> 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 >> > > v
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > 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 > 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.replica
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 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
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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/ 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 > > wrote: > > > > > Hi Rajini, > > > > > > Just to clarify, are you proposing this as a way to co
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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/ 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 > 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 > > 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 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 > > wrote: > > > >> > > > > >> > > bq. There is a --de
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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/ 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 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 > 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 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 > 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 > >> > > ?> > > >> > > > configs)* will be invoked with all of the configured configs of > > the > > >> > > broker, > > >> > > > similar to *configure(Map 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 > > >> wrote: > > >> > > > > > >> > > > > bq. (e.g. increase network/IO threads) > > >> > > > > > > >> > > > > Would decreasing network/IO threads be supported ? > > >> > > > > > > >> > > > > bq. void reconfigure(Map configs); > > >> > > > > > > >> > > > > What if some keys in configs are not in the Set returned by > > >> > > > > reconfigu
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > 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 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 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 >> > > ?> > >> > > > configs)* will be invoked with all of the configured configs of > the > >> > > broker, > >> > > > similar to *configure(Map 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 > >> wrote: > >> > > > > >> > > > > bq. (e.g. increase network/IO threads) > >> > > > > > >> > > > > Would decreasing network/IO threads be supported ? > >> > > > > > >> > > > > bq. void reconfigure(Map 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: > >> > > > > > >>
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 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 > > >> wrote: >> >> > Ted, >> > >> > Have added an example for --delete-config. >> > >> > On Mon, Nov 20, 2017 at 7:42 PM, Ted Yu 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> > > ?> >> > > > configs)* will be invoked with all of the configured configs of the >> > > broker, >> > > > similar to *configure(Map 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 >> wrote: >> > > > >> > > > > bq. (e.g. increase network/IO threads) >> > > > > >> > > > > Would decreasing network/IO threads be supported ? >> > > > > >> > > > > bq. void reconfigure(Map 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
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > wrote: > > > Ted, > > > > Have added an example for --delete-config. > > > > On Mon, Nov 20, 2017 at 7:42 PM, Ted Yu 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 > > ?> > > > > configs)* will be invoked with all of the configured configs of the > > > broker, > > > > similar to *configure(Map 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 wrote: > > > > > > > > > bq. (e.g. increase network/IO threads) > > > > > > > > > > Would decreasing network/IO threads be supported ? > > > > > > > > > > bq. void reconfigure(Map 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 > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 wrote: > Ted, > > Have added an example for --delete-config. > > On Mon, Nov 20, 2017 at 7:42 PM, Ted Yu 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 > ?> > > > configs)* will be invoked with all of the configured configs of the > > broker, > > > similar to *configure(Map 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 wrote: > > > > > > > bq. (e.g. increase network/IO threads) > > > > > > > > Would decreasing network/IO threads be supported ? > > > > > > > > bq. void reconfigure(Map 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 > > > > > > > > > > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Ted, Have added an example for --delete-config. On Mon, Nov 20, 2017 at 7:42 PM, Ted Yu 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 > 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 ?> > > configs)* will be invoked with all of the configured configs of the > broker, > > similar to *configure(Map 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 wrote: > > > > > bq. (e.g. increase network/IO threads) > > > > > > Would decreasing network/IO threads be supported ? > > > > > > bq. void reconfigure(Map 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 > > > > > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 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 > configs)* will be invoked with all of the configured configs of the broker, > similar to *configure(Map 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 wrote: > > > bq. (e.g. increase network/IO threads) > > > > Would decreasing network/IO threads be supported ? > > > > bq. void reconfigure(Map 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 > > > 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 > > > > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
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 configs)* will be invoked with all of the configured configs of the broker, similar to *configure(Map 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 wrote: > bq. (e.g. increase network/IO threads) > > Would decreasing network/IO threads be supported ? > > bq. void reconfigure(Map 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 > 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 > > >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
bq. (e.g. increase network/IO threads) Would decreasing network/IO threads be supported ? bq. void reconfigure(Map 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 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 >
Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration
Hi, can you help me?
i have a problem, i would like to create an avro schema on schema registry
of kafka that references a type that is declared in a different avro
schema, the places under an example:
First avro schema with reference UserAction
{"namespace": "com.myorg.other",
"type": "record",
"name": "SearchSuggest",
"fields": [
{"name": "name", "type": "string"},
{"name": "userAction", "type": "UserAction"}
]
}
second avro schema with enum:
{"namespace": "com.myorg.other",
"type": "enum",
"name": "UserAction",
"symbols": ["S", "V", "C"]
}
This work in my maven project, but when i try create this on schema
registry, it's invalid.
2017-11-20 14:57 GMT+01:00 Rajini Sivaram :
> 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
>
[DISCUSS] KIP 226 - Dynamic Broker Configuration
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
