Colin/Ewen, i will add changes to bump the API version.
any preferences on the return type for the new method? tbh it seems like returning a NewTopic could make sense because the ConfigResource for a TOPIC type does not let me encode `numPartitions` thanks dan On Mon, Dec 11, 2017 at 7:22 PM, Colin McCabe <cmcc...@apache.org> wrote: > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
