kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin McCabe <cmcc...@apache.org>
Subject Re: [DISCUSS] KIP-234: add support for getting topic defaults from AdminClient
Date Tue, 12 Dec 2017 03:22:36 GMT
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.norwood@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 <lindong28@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.norwood@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 <
> > rajinisivaram@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.norwood@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 <lindong28@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.norwood@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 <lindong28@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.norwood@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
<
> > > lindong28@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.norwood@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
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >

Mime
View raw message