Hi Dan, The KIP looks good overall.
On Mon, Dec 11, 2017, at 18:28, Ewen Cheslack-Postava wrote: > I think the key point is when the kafka admin and user creating topics > differ. I think a more realistic example of Dan's point (2) is for > retention. I know that realistically, admins aren't just going to > randomly > drop the broker defaults from 1w to 1d without warning anyone (they'd > likely be fired...). But as a user, I may not know the broker configs, if > admins have overridden them, etc. I may want a *minimum* of, e.g., 2d. > But if the broker defaults are higher such that the admins are confident the > cluster can handle 1w, I'd rather just fall back on the default value. Right. I think this API addresses a similar set of use-cases as adding the "validateOnly" boolean for createTopics. You shouldn't have to create a topic to know whether it was possible to create it, or what the retention will end up being, etc. etc. > Now, there's arguably a better solution for that case -- allow topic > configs to express a *minimum* value (or maximum depending on the > particular config), with the broker config taking precedence if it has a > smaller value (or larger in the case of maximums). This lets you express > your minimum requirements but allows the cluster to do more if that's the > default. However, that would represent a much more significant and > invasive change, and honestly I think it is more likely to confuse users. There always need to be topic defaults, though. If we add a foobar configuration for topics, existing topics will need to get grandfathered in with a default foobar. And they won't be able to set min and max ranges, because foobars didn't exist back when the old topics were created. > > @Dan, regarding compatibility, this changes behavior without revving the > request version number, which normally we only do for things that are > reasonably considered bugfixes or were it has no compatibility > implications. In this case, older brokers talking to newer AdminClients > will presumably return some error. Do we know what the non-null assertion > gets converted to and if we're happy with the behavior (i.e. will > applications be able to do something reasonable, distinguish it from some > completely unrelated error, etc)? Similarly, it's obviously only one > implementation using the KIP-4 APIs, but do we know what client-side > validation AdminClient is already doing and whether old AdminClients > talking to new brokers will see a change in behavior (or do they do > client-side validation such that old clients simply wouldn't have access > to this new functionality)? I think we should bump the API version for this or add a new API key. Nothing good is going to happen by pretending like this is compatible with existing brokers. Also, I think it would be better just to have a separate function in AdminClient rather than overloading the behavior of NULL in describeConfigs. It's not really that much more effort to have another AdminClient function, and it's a lot simpler for devs to understand than magical NULL behavior in describeConfigs. best, Colin > > -Ewen > > On Mon, Dec 11, 2017 at 2:11 PM, dan <dan.norw...@gmail.com> wrote: > > > Dong, > > > > I agree that it *may* be better for a user to be explicit, however there > > are a couple reasons they may not. > > 1) a user doesn't even know what the options are. imagine writing a tool > > for users to create topics that steps them through things: > > > > $ kafka-topics.sh --create > > Give your topic a name: my-fav-topic > > How many partitions do you want [12]: > > What is the minimum in set replica size [2]: > > What is the maximum message size [1MB]: > > ... > > > > 2) a user wants to use broker defaults within reason. say they thinke they > > want min.cleanable.dirty.ratio=.5 and the default is .6. maybe thats fine, > > or even better for them. since the person maintaining the actual cluster > > has put thought in to this config. and as the maintainer keeps working on > > making the cluster run better they can change and tune things on the > > cluster level as needed. > > > > dan > > > > > > On Wed, Dec 6, 2017 at 11:51 AM, Dong Lin <lindon...@gmail.com> wrote: > > > > > Hey Dan, > > > > > > I think you are saying that, if user can read the default config before > > > creating the topic, then this user can make better decision in what > > configs > > > need to be overwritten. The question here is, how user is going to use > > this > > > config to make the decision. > > > > > > In my understanding, user will compare the default value with expected > > > value, and override the config to be expected value if they are > > different. > > > If this is the only way that the default value can affect user's > > decision, > > > then it seems OK for user to directly override the config to the expected > > > value. I am wondering if this solution has some drawback. > > > > > > On the other hand, maybe there is a more advanced way that the default > > > value can affect the user's decision. It may be useful to understand this > > > use-case more specifically. Could you help provide a specific example > > here? > > > > > > Thanks, > > > Dong > > > > > > > > > On Wed, Dec 6, 2017 at 11:12 AM, dan <dan.norw...@gmail.com> wrote: > > > > > > > Rajini, > > > > > > > > that was not my intent, the intent was to give a user of this api an > > > > insight in to what their topic will look like once created. as things > > > stand > > > > now a user is unable to (easily) have any knowledge of what their topic > > > > configs will be before doing a `CREATE_TOPICS`. as i mentioned in the > > > KIP, > > > > another option would be to have the `CreateTopicsOptions. > > > > validateOnly=true` > > > > version return data, but seems more invasive/questionable. > > > > > > > > dan > > > > > > > > On Wed, Dec 6, 2017 at 5:10 AM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Dan, > > > > > > > > > > Thank you for the KIP. KIP-226 (https://cwiki.apache.org/ > > > > > confluence/display/KAFKA/KIP-226+-+Dynamic+Broker+Configuration) > > > > proposes > > > > > to add an option to include all synonyms of a config option when > > > > describing > > > > > configs. This includes any defaults. For example (using Dong's > > > example), > > > > if > > > > > you have topicA with min.cleanable.dirty.ratio=0.6 as an override and > > > the > > > > > broker default is 0.5, AdminClient#describeConfigs with synonyms > > would > > > > > return the actual value in use as the config value (I,e. > > > > > min.cleanable.dirty.ratio=0.6). And the synonyms list would contain > > > (in > > > > > the > > > > > order of precedence in which these configs are applied): > > > > > > > > > > 1. min.cleanable.dirty.ratio=0.6, config source=TOPIC_CONFIG > > > > > 2. log.min.cleanable.dirty.ratio=0.5, config > > > > > source=STATIC_BROKER_CONFIG > > > > > > > > > > > > > > > KIP-226 doesn't give you exactly what you are proposing in this KIP, > > > but > > > > it > > > > > gives the mapping of configs. My concern with this KIP is that it > > > assumes > > > > > that if the broker config is static, i.e. if the current value of > > > > > log.min.cleanable.dirty.ratio=0.6, you can safely create a topic > > with > > > > > default min.cleanable.dirty.ratio relying on that the value to be > > > applied > > > > > all the time. This will not work with dynamic broker configs if the > > > > broker > > > > > defaults can be updated at any time. > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 11:22 PM, dan <dan.norw...@gmail.com> wrote: > > > > > > > > > > > for point 1 i agree, its not too strong. only addition i could come > > > up > > > > > with > > > > > > is that it allows any utility to have better forwards > > compatability. > > > a > > > > > cli > > > > > > written that can inspect how a topic will be created would be able > > to > > > > > give > > > > > > insight/expectations about configs that didn't exist at compilation > > > > time. > > > > > > > > > > > > for point 2, i am thinking about a situation where the user > > creating > > > > > topics > > > > > > and the user managing brokers are different people with different > > > > > > skills/abilities. > > > > > > > > > > > > so in the given example lets assume a user creating the topic does > > > not > > > > > > *really* understand what that config means, so they are likely to > > > just > > > > > > leave it as default. and are happy for their admin to change these > > on > > > > the > > > > > > broker as needed. > > > > > > > > > > > > but say we have another user who is creating a topic who has much > > > more > > > > > > experience and has done testing, they will be able to determine > > what > > > > the > > > > > > value is on the cluster and check to see if it matches expectations > > > or > > > > > > needs to be set. possibly if this is set to something incorrect for > > > > their > > > > > > usecase they will have a reason to verify and speak with the admin > > > > about > > > > > > their usecase. > > > > > > > > > > > > > > > > > > moreover, i think without this added capability it makes it > > > > > > difficult/impossible to accurately use broker defaults for topics. > > > > right > > > > > > now a user is left to either decide configs a priori and lose this > > > > > > functionality, or guess/check what they need to set and end in a > > > > possibly > > > > > > bad situation until they can get their *live* topic configured. > > > > > > > > > > > > dan > > > > > > > > > > > > On Mon, Dec 4, 2017 at 2:50 PM, Dong Lin <lindon...@gmail.com> > > > wrote: > > > > > > > > > > > > > Hey Dan, > > > > > > > > > > > > > > Thanks again for the update:) > > > > > > > > > > > > > > I am not sure I fully understand the points (1) and (2) in the > > > > "Always > > > > > > > Configure ALL Configs For a Topic". In my previous question, I > > > don't > > > > > mean > > > > > > > that users should specify full list of topics configs. I mean > > that > > > > user > > > > > > can > > > > > > > specify the full list of topic configs he/she wants to override. > > > > > > > > > > > > > > Your KIP proposes to allow user to query the default topic > > configs. > > > > In > > > > > my > > > > > > > understanding, users want to know the default configs only if > > > he/she > > > > > > > already has a specific list of (config, value) pairs for which > > > he/she > > > > > > wants > > > > > > > to override. The workflow will be that user queries the default > > > > > configs, > > > > > > > compares the default value with that specific list, and > > selectively > > > > > > > override the configs whose value is different from the default > > > value. > > > > > > > Therefore, the alternative solution does not suffer the problem > > (1) > > > > > > because > > > > > > > user needs to know that specific list of (config, value) pair > > > anyway. > > > > > > Does > > > > > > > this make sense? > > > > > > > > > > > > > > Regarding problem (2), I think you are saying that if the user > > > wants > > > > to > > > > > > set > > > > > > > the min.cleanable.dirty.ratio to be 0.5 and the default value is > > > > > > currently > > > > > > > set to 0.5, then user would not want to explicitly override the > > > > config > > > > > > > during topic creation. The purpose is for the > > > > min.cleanable.dirty.ratio > > > > > > of > > > > > > > this topic to be changed to 0.6 if admin change the default > > > > > > > min.cleanable.dirty.ratio to 0.6 in the future. > > > > > > > > > > > > > > But I am not sure this is a reasonable example. If user wants to > > > > > > > compare default value of min.cleanable.dirty.ratio with its > > > expected > > > > > > value > > > > > > > 0.5, then it seems reasonable for user to override it to be 0.5 > > > > during > > > > > > > topic creation if the default value is currently 0.6. The > > question > > > > is, > > > > > > why > > > > > > > would user not want to override the default value to be 0.5 if > > the > > > > > > default > > > > > > > value is changed from 0.5 to 0.6 later? > > > > > > > > > > > > > > Thanks, > > > > > > > Dong > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 2:36 PM, dan <dan.norw...@gmail.com> > > wrote: > > > > > > > > > > > > > > > updated again :) > > > > > > > > > > > > > > > > by having users always set all configs you lose the ability to > > > use > > > > > the > > > > > > > > broker defaults as intended, since topic configs are overlaid. > > > > > example > > > > > > in > > > > > > > > the kip doc. > > > > > > > > > > > > > > > > dan > > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 11:47 AM, Dong Lin <lindon...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hey Dan, > > > > > > > > > > > > > > > > > > Thanks for the update. I just want to push the discussion a > > bit > > > > > > > further. > > > > > > > > > Another alternative, which currently is not described in the > > > KIP, > > > > > is > > > > > > > for > > > > > > > > > user to always create the topic with the full list of configs > > > it > > > > > may > > > > > > > want > > > > > > > > > to override. Can you help me understand what is the drawback > > of > > > > > this > > > > > > > > > approach? > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Dong > > > > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 11:30 AM, dan <dan.norw...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > > > > Dong, > > > > > > > > > > > > > > > > > > > > i added a section on current state and workarounds along > > with > > > > my > > > > > > > > > arguments > > > > > > > > > > for why they are less than optimal to the wiki. but the > > jist > > > of > > > > > it > > > > > > is > > > > > > > > you > > > > > > > > > > can end up with messages in your topic in an > > > incorrect/invalid > > > > > > state > > > > > > > if > > > > > > > > > you > > > > > > > > > > do this. > > > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > dan > > > > > > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 10:53 AM, Dong Lin < > > > lindon...@gmail.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hey Dan, > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP. Can you help me understand the > > > motivation > > > > > by > > > > > > > > > > providing > > > > > > > > > > > a use-case that can not be easily completed without this > > > KIP? > > > > > > > > > > > > > > > > > > > > > > It seems that most users will simply create the topic > > > without > > > > > > > > worrying > > > > > > > > > > > about the default configs. If a user has specific > > > requirement > > > > > for > > > > > > > the > > > > > > > > > > > default configs, he/she can use > > > AdminClient.describeConfigs() > > > > > and > > > > > > > > > > > AdminClient.alterConfigs() after the topic has been > > > created. > > > > > This > > > > > > > > seems > > > > > > > > > > to > > > > > > > > > > > work well. Did I miss something? > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > Dong > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 4, 2017 at 9:25 AM, dan < > > dan.norw...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > I would like to start a discussion about KIP-234 > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > > > > > > 234%3A+add+support+for+getting+topic+defaults+from+ > > > > > AdminClient > > > > > > > > > > > > > > > > > > > > > > > > thanks > > > > > > > > > > > > dan > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
