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 <ja...@confluent.io> 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 <rajinisiva...@gmail.com> > 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<String, ?> configs) > > and reconfigure(Map<String, > > ?> 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 <ja...@confluent.io> > > 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 < > > 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 <j...@confluent.io> 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 < > > > rajinisiva...@gmail.com > > > > > > > > > > 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 <j...@confluent.io> > 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 > > > > > > > > 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 <j...@confluent.io> > > > 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 <j...@confluent.io> > > > > 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<String, > > > > > > > ?> > > > > > > > > > > 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 < > > > > > cmcc...@apache.org> > > > > > > > > > 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 < > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > >> 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 < > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > >> > > wrote: > > > > > > > > > >> > > > > > > > > > > > > >> > > > > Hi Rajini, > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > This seems like a nice improvement! > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > One thing that is a bit concerning is that, if > > > > > > > > > >> bin/kafka-configs.sh is > > > > > > > > > >> > > > > used, there is no way for the broker to give > > > feedback > > > > > or > > > > > > > > error > > > > > > > > > >> > > > > messages. The broker can't say "sorry, I can't > > > > > > reconfigure > > > > > > > > that > > > > > > > > > >> in > > > > > > > > > >> > > that > > > > > > > > > >> > > > > way." Or even "that configuration property is > not > > > > > > > > > reconfigurable > > > > > > > > > >> in > > > > > > > > > >> > > > > this version of the software." It seems like in > > the > > > > > > > examples > > > > > > > > > >> give, > > > > > > > > > >> > > > > users will simply set a configuration using > > > > > > > > > bin/kafka-configs.sh, > > > > > > > > > >> but > > > > > > > > > >> > > > > then they have to check the broker log files to > > see > > > if > > > > > it > > > > > > > > could > > > > > > > > > >> > > actually > > > > > > > > > >> > > > > be applied. And even if it couldn't be applied, > > > then > > > > it > > > > > > > still > > > > > > > > > >> lingers > > > > > > > > > >> > > > > in ZooKeeper. > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > This seems like it would lead to a lot of user > > > > > confusion, > > > > > > > > since > > > > > > > > > >> they > > > > > > > > > >> > > get > > > > > > > > > >> > > > > no feedback when reconfiguring something. For > > > > example, > > > > > > > there > > > > > > > > > >> will be a > > > > > > > > > >> > > > > lot of scenarios where someone finds a > > > reconfiguration > > > > > > > command > > > > > > > > > on > > > > > > > > > >> > > > > Google, runs it, but then it doesn't do anything > > > > because > > > > > > the > > > > > > > > > >> software > > > > > > > > > >> > > > > version is different. And there's no error > > message > > > or > > > > > > > > feedback > > > > > > > > > >> about > > > > > > > > > >> > > > > this. It just doesn't work. > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > To prevent this, I think we should convert > > > > > > > > bin/kafka-configs.sh > > > > > > > > > >> to use > > > > > > > > > >> > > > > AdminClient's AlterConfigsRequest. This allows > us > > > to > > > > > > detect > > > > > > > > > >> scenarios > > > > > > > > > >> > > > > where, because of a typo, different software > > > version, > > > > > or a > > > > > > > > value > > > > > > > > > >> of the > > > > > > > > > >> > > > > wrong type (eg. string vs. int), the given > > > > configuration > > > > > > > > cannot > > > > > > > > > be > > > > > > > > > >> > > > > applied. We really should convert > > kafka-configs.sh > > > to > > > > > use > > > > > > > > > >> AdminClient > > > > > > > > > >> > > > > anyway, for all the usual reasons-- people want > to > > > > lock > > > > > > down > > > > > > > > > >> ZooKeeper, > > > > > > > > > >> > > > > ACLs should be enforced, internal > representations > > > > should > > > > > > be > > > > > > > > > >> hidden, we > > > > > > > > > >> > > > > should support environments where ZK is not > > exposed, > > > > > etc. > > > > > > > etc. > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > Another issue that I see here is, how does this > > > > interact > > > > > > > with > > > > > > > > > >> > > downgrade? > > > > > > > > > >> > > > > Presumably, if the user downgrades to a version > > > that > > > > > > > doesn't > > > > > > > > > >> support > > > > > > > > > >> > > > > KIP-226, all the dynamic configurations stored > in > > ZK > > > > > > revert > > > > > > > to > > > > > > > > > >> whatever > > > > > > > > > >> > > > > value they have in the local config files. Do > we > > > need > > > > > to > > > > > > > > have a > > > > > > > > > >> > > utility > > > > > > > > > >> > > > > that can reify the actual applied configuration > > > into a > > > > > > local > > > > > > > > > text > > > > > > > > > >> file, > > > > > > > > > >> > > > > to make downgrades less painful? > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > With regard to upgrades, what happens if we > change > > > the > > > > > > name > > > > > > > > of a > > > > > > > > > >> > > > > configuration key in the future? For example, > if > > we > > > > > > decide > > > > > > > to > > > > > > > > > >> rename > > > > > > > > > >> > > > > min.insync.replicas to min.in.sync.replicas, > > > > presumably > > > > > we > > > > > > > > will > > > > > > > > > >> > > > > deprecate the old key name. And then perhaps it > > > will > > > > be > > > > > > > > removed > > > > > > > > > >> in a > > > > > > > > > >> > > > > future release, such as Apache Kafka 2.0. In > this > > > > > > scenario, > > > > > > > > > >> should the > > > > > > > > > >> > > > > Kafka upgrade process change the name of the > > > > > configuration > > > > > > > key > > > > > > > > > in > > > > > > > > > >> ZK > > > > > > > > > >> > > > > from min.insync.replicas to > min.in.sync.replicas? > > > > > > Obviously > > > > > > > > > this > > > > > > > > > >> will > > > > > > > > > >> > > > > make downgrades harder, if so. But if it > doesn't, > > > > then > > > > > > > > removing > > > > > > > > > >> > > > > deprecated configuration key synonyms might > become > > > > very > > > > > > > > > difficult. > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > best, > > > > > > > > > >> > > > > Colin > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > On Wed, Nov 22, 2017, at 12:52, Rajini Sivaram > > > wrote: > > > > > > > > > >> > > > > > Hi Tom, > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > No, I am not proposing this as a way to > > configure > > > > > > > > replication > > > > > > > > > >> quotas. > > > > > > > > > >> > > > > > When > > > > > > > > > >> > > > > > you describe broker configs using AdminClient, > > you > > > > > will > > > > > > > see > > > > > > > > > all > > > > > > > > > >> the > > > > > > > > > >> > > > > > configs > > > > > > > > > >> > > > > > persisted in /configs/brokers/<brokerId> in > > > > ZooKeeper > > > > > > and > > > > > > > > this > > > > > > > > > >> > > includes > > > > > > > > > >> > > > > > leader.replication.throttled.rate, > > > > > > follower.replication. > > > > > > > > > >> > > throttled.rate > > > > > > > > > >> > > > > > etc. But > > > > > > > > > >> > > > > > the broker configs that can be altered using > > > > > AdminClient > > > > > > > as > > > > > > > > a > > > > > > > > > >> result > > > > > > > > > >> > > of > > > > > > > > > >> > > > > > this KIP are those explicitly stated in the > KIP > > > > (does > > > > > > not > > > > > > > > > >> include > > > > > > > > > >> > > any of > > > > > > > > > >> > > > > > the quota configs). > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > Regards, > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > Rajini > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > On Wed, Nov 22, 2017 at 7:54 PM, Tom Bentley < > > > > > > > > > >> t.j.bent...@gmail.com> > > > > > > > > > >> > > > > > wrote: > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > Hi Rajini, > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > Just to clarify, are you proposing this as a > > way > > > > to > > > > > > > > > configure > > > > > > > > > >> > > > > interbroker > > > > > > > > > >> > > > > > > throttling/quotas? I don't think you are, > just > > > > > wanted > > > > > > to > > > > > > > > > check > > > > > > > > > >> > > (since > > > > > > > > > >> > > > > > > KIP-179 proposes a different mechanism for > > > setting > > > > > > them > > > > > > > > > which > > > > > > > > > >> > > supports > > > > > > > > > >> > > > > > > their automatic removal). > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > Cheers, > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > Tom > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > On 22 November 2017 at 18:28, Rajini > Sivaram < > > > > > > > > > >> > > rajinisiva...@gmail.com> > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > I have made an update to the KIP to > > optionally > > > > > > return > > > > > > > > all > > > > > > > > > >> the > > > > > > > > > >> > > config > > > > > > > > > >> > > > > > > > synonyms in *DescribeConfigsResponse*. The > > > > > synonyms > > > > > > > are > > > > > > > > > >> returned > > > > > > > > > >> > > in > > > > > > > > > >> > > > > the > > > > > > > > > >> > > > > > > > order of precedence. AlterConfigsResponse > > will > > > > not > > > > > > be > > > > > > > > > >> modified > > > > > > > > > >> > > by the > > > > > > > > > >> > > > > > > KIP. > > > > > > > > > >> > > > > > > > Since many configs already have various > > > > overrides > > > > > > > (e.g. > > > > > > > > > >> topic > > > > > > > > > >> > > configs > > > > > > > > > >> > > > > > > with > > > > > > > > > >> > > > > > > > broker overrides, security properties with > > > > > listener > > > > > > > name > > > > > > > > > >> > > overrides) > > > > > > > > > >> > > > > and > > > > > > > > > >> > > > > > > we > > > > > > > > > >> > > > > > > > will be adding more levels with dynamic > > > configs, > > > > > it > > > > > > > will > > > > > > > > > be > > > > > > > > > >> > > useful to > > > > > > > > > >> > > > > > > > obtain the full list in order of > precedence. > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > On Tue, Nov 21, 2017 at 11:24 AM, Rajini > > > > Sivaram < > > > > > > > > > >> > > > > > > rajinisiva...@gmail.com> > > > > > > > > > >> > > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > Hi Ted, > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > You can quote the config name, but it is > > not > > > > > > > necessary > > > > > > > > > for > > > > > > > > > >> > > > > deleting a > > > > > > > > > >> > > > > > > > > config since the name doesn't contain > any > > > > > special > > > > > > > > > >> characters > > > > > > > > > >> > > that > > > > > > > > > >> > > > > > > > requires > > > > > > > > > >> > > > > > > > > quoting. > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > On Mon, Nov 20, 2017 at 9:20 PM, Ted Yu > < > > > > > > > > > >> yuzhih...@gmail.com> > > > > > > > > > >> > > > > wrote: > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > >> Thanks for the quick response. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > >> It seems the config following > > > --delete-config > > > > > > > should > > > > > > > > be > > > > > > > > > >> > > quoted. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > >> Cheers > > > > > > > > > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > >> On Mon, Nov 20, 2017 at 12:02 PM, > Rajini > > > > > Sivaram > > > > > > < > > > > > > > > > >> > > > > > > > rajinisiva...@gmail.com > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > >> > > > > > > > >> wrote: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > Ted, > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > >> > > > > > > > >> > Have added an example for > > > --delete-config. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > >> > > > > > > > >> > On Mon, Nov 20, 2017 at 7:42 PM, Ted > > Yu < > > > > > > > > > >> > > yuzhih...@gmail.com> > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > >> > > > > > > > >> > > bq. There is a --delete-config > option > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > Consider adding a sample with the > > above > > > > > > option > > > > > > > to > > > > > > > > > >> the KIP. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > Thanks > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > On Mon, Nov 20, 2017 at 11:36 AM, > > > Rajini > > > > > > > Sivaram > > > > > > > > < > > > > > > > > > >> > > > > > > > >> > rajinisiva...@gmail.com> > > > > > > > > > >> > > > > > > > >> > > wrote: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > > Hi Ted, > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > Thank you for reviewing the KIP. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > *Would decreasing network/IO > > threads > > > be > > > > > > > > supported > > > > > > > > > >> ?* > > > > > > > > > >> > > > > > > > >> > > > Yes, As described in the KIP, > some > > > > > > > connections > > > > > > > > > >> will be > > > > > > > > > >> > > > > closed if > > > > > > > > > >> > > > > > > > >> > network > > > > > > > > > >> > > > > > > > >> > > > thread count is reduced (and > > > > > reconnections > > > > > > > will > > > > > > > > > be > > > > > > > > > >> > > > > processed on > > > > > > > > > >> > > > > > > > >> > remaining > > > > > > > > > >> > > > > > > > >> > > > threads) > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > *What if some keys in configs are > > not > > > > in > > > > > > the > > > > > > > > Set > > > > > > > > > >> > > returned > > > > > > > > > >> > > > > > > > >> > > > by reconfigurableConfigs()? Would > > > > > exception > > > > > > > be > > > > > > > > > >> thrown ?* > > > > > > > > > >> > > > > > > > >> > > > No, *reconfigurableConfigs() > *will > > be > > > > > used > > > > > > to > > > > > > > > > >> decide > > > > > > > > > >> > > which > > > > > > > > > >> > > > > > > classes > > > > > > > > > >> > > > > > > > >> are > > > > > > > > > >> > > > > > > > >> > > > notified when a configuration > > update > > > is > > > > > > > made*. > > > > > > > > > >> > > > > > > > >> > **reconfigure(Map<String, > > > > > > > > > >> > > > > > > > >> > > ?> > > > > > > > > > >> > > > > > > > >> > > > configs)* will be invoked with > all > > of > > > > the > > > > > > > > > >> configured > > > > > > > > > >> > > > > configs of > > > > > > > > > >> > > > > > > > the > > > > > > > > > >> > > > > > > > >> > > broker, > > > > > > > > > >> > > > > > > > >> > > > similar to > *configure(Map<String, > > > ?> > > > > > > > > configs). > > > > > > > > > >> *For > > > > > > > > > >> > > > > example, > > > > > > > > > >> > > > > > > > when > > > > > > > > > >> > > > > > > > >> > > > *SslChannelBuilder* is made > > > > > reconfigurable, > > > > > > > it > > > > > > > > > >> could > > > > > > > > > >> > > just > > > > > > > > > >> > > > > > > create a > > > > > > > > > >> > > > > > > > >> new > > > > > > > > > >> > > > > > > > >> > > > SslFactory with the latest > configs, > > > > using > > > > > > the > > > > > > > > > same > > > > > > > > > >> code > > > > > > > > > >> > > as > > > > > > > > > >> > > > > > > > >> > *configure()*. > > > > > > > > > >> > > > > > > > >> > > > We avoid reconfiguring > > > > *SslChannelBuilder > > > > > > > > > >> > > *unnecessarily*, > > > > > > > > > >> > > > > *for > > > > > > > > > >> > > > > > > > >> example > > > > > > > > > >> > > > > > > > >> > > if > > > > > > > > > >> > > > > > > > >> > > > a topic config has changed, since > > > topic > > > > > > > configs > > > > > > > > > >> are not > > > > > > > > > >> > > > > listed > > > > > > > > > >> > > > > > > in > > > > > > > > > >> > > > > > > > >> the > > > > > > > > > >> > > > > > > > >> > > > *SslChannelBuilder#** > > > > > > > reconfigurableConfigs().* > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > *The sample commands for > > > > > bin/kafka-configs > > > > > > > > > include > > > > > > > > > >> > > > > > > '--add-config'. > > > > > > > > > >> > > > > > > > >> > Would > > > > > > > > > >> > > > > > > > >> > > > there be '--remove-config' ?* > > > > > > > > > >> > > > > > > > >> > > > bin/kafka-configs.sh is an > existing > > > > tool > > > > > > > whose > > > > > > > > > >> > > parameters > > > > > > > > > >> > > > > will > > > > > > > > > >> > > > > > > not > > > > > > > > > >> > > > > > > > >> be > > > > > > > > > >> > > > > > > > >> > > > modified by this KIP. There is a > > > > > > > > --delete-config > > > > > > > > > >> option. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > *ssl.keystore.password appears a > > few > > > > > lines > > > > > > > > above. > > > > > > > > > >> Would > > > > > > > > > >> > > > > there be > > > > > > > > > >> > > > > > > > any > > > > > > > > > >> > > > > > > > >> > > > issue with mixture of connections > > > (with > > > > > old > > > > > > > and > > > > > > > > > new > > > > > > > > > >> > > > > password) ?* > > > > > > > > > >> > > > > > > > >> > > > No, passwords (and the actual > > > keystore) > > > > > are > > > > > > > > only > > > > > > > > > >> used > > > > > > > > > >> > > during > > > > > > > > > >> > > > > > > > >> > > > authentication. Any channel > created > > > > using > > > > > > the > > > > > > > > old > > > > > > > > > >> > > SslFactory > > > > > > > > > >> > > > > > > will > > > > > > > > > >> > > > > > > > >> not > > > > > > > > > >> > > > > > > > >> > be > > > > > > > > > >> > > > > > > > >> > > > impacted. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > Regards, > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > Rajini > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > On Mon, Nov 20, 2017 at 4:39 PM, > > Ted > > > > Yu < > > > > > > > > > >> > > > > yuzhih...@gmail.com> > > > > > > > > > >> > > > > > > > >> wrote: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > > bq. (e.g. increase network/IO > > > > threads) > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > Would decreasing network/IO > > threads > > > > be > > > > > > > > > supported > > > > > > > > > >> ? > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > bq. void > > > reconfigure(Map<String, > > > > ?> > > > > > > > > > configs); > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > What if some keys in configs > are > > > not > > > > in > > > > > > the > > > > > > > > Set > > > > > > > > > >> > > returned > > > > > > > > > >> > > > > by > > > > > > > > > >> > > > > > > > >> > > > > reconfigurableConfigs() > > > > > > > > > >> > > > > > > > >> > > > > ? Would exception be thrown ? > > > > > > > > > >> > > > > > > > >> > > > > If so, please specify which > > > exception > > > > > > would > > > > > > > > be > > > > > > > > > >> thrown. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > The sample commands for > > > > > bin/kafka-configs > > > > > > > > > include > > > > > > > > > >> > > > > > > > '--add-config'. > > > > > > > > > >> > > > > > > > >> > > > > Would there be > '--remove-config' > > ? > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > bq. Existing connections will > not > > > be > > > > > > > > affected, > > > > > > > > > >> new > > > > > > > > > >> > > > > connections > > > > > > > > > >> > > > > > > > >> will > > > > > > > > > >> > > > > > > > >> > use > > > > > > > > > >> > > > > > > > >> > > > the > > > > > > > > > >> > > > > > > > >> > > > > new keystore. > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > ssl.keystore.password appears a > > few > > > > > lines > > > > > > > > > above. > > > > > > > > > >> Would > > > > > > > > > >> > > > > there > > > > > > > > > >> > > > > > > be > > > > > > > > > >> > > > > > > > >> any > > > > > > > > > >> > > > > > > > >> > > issue > > > > > > > > > >> > > > > > > > >> > > > > with mixture of connections > (with > > > old > > > > > and > > > > > > > new > > > > > > > > > >> > > password) ? > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > Cheers > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > On Mon, Nov 20, 2017 at 5:57 > AM, > > > > Rajini > > > > > > > > > Sivaram < > > > > > > > > > >> > > > > > > > >> > > rajinisiva...@gmail.com > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > wrote: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > Hi all, > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > >> > > > > > I have submitted KIP-226 to > > > enable > > > > > > > dynamic > > > > > > > > > >> > > > > reconfiguration > > > > > > > > > >> > > > > > > of > > > > > > > > > >> > > > > > > > >> > brokers > > > > > > > > > >> > > > > > > > >> > > > > > without restart: > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > >> > > > > > https://cwiki.apache.org/ > > > > > > > > > >> > > confluence/display/KAFKA/KIP- > > > > > > > > > >> > > > > > > > >> > > > > > > 226+-+Dynamic+Broker+Configura > > > tion > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > >> > > > > > 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 > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
