kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guozhang Wang <wangg...@gmail.com>
Subject Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different consumers
Date Mon, 02 Apr 2018 18:25:29 GMT
@Matthias

That's a good question: I think adding another config for the main consumer
makes good tradeoffs between compatibility and new user convenience. Just
to clarify, from user's pov on upgrade:

1) I'm already overriding some consumer configs, and now I want to override
these values differently for restore consumers, I'd add one new line for
the restore consumer prefix.

2) I'm already overriding some consumer configs, and now I want to NOT
overriding them for restore consumers, I'd change my override from
`consumer.X` to `main.consumer.X`.

3) I'm new and have not any consumer overrides, and now if I want to
override some, I'd use `main.consumer`, `restore.consumer` for specific
consumer types, and ONLY consider `consumer` for the ones that I want to
apply universally.

4) I'm already overriding some consumer configs and I'm happy with what I
get, I do not change anything.


Guozhang

On Mon, Apr 2, 2018 at 11:10 AM, Ted Yu <yuzhihong@gmail.com> wrote:

> bq. to introduce one more prefix `main.consumer.`
>
> Makes sense.
>
> On Mon, Apr 2, 2018 at 11:06 AM, Matthias J. Sax <matthias@confluent.io>
> wrote:
>
> > Boyang,
> >
> > thanks a lot for the KIP.
> >
> > Couple of questions:
> >
> > > (MODIFIED) public Map<String, Object> getRestoreConsumerConfigs(final
> > String clientId);
> >
> > You mean that the implementation/semantics of this method changes, but
> > not the API itself? Might be good to add this as "comment style" instead
> > of "(MODIFIED)" prefix.
> >
> > > By rewriting the getRestoreConsumerConfigs() function and adding the
> > getGlobalConsumerConfigs() function, if one user uses
> > restoreConsumerPrefix() or globalConsumerPrefix() when adding new
> > configurations, the configs shall overwrite base consumer config. If not
> > specified, restore consumer and global consumer shall share the same
> config
> > with base consumer.
> >
> > While this does make sense for backward compatibility, I am wonder if it
> > makes the config "inheritance logic" (ie, hierarchy) too complex? We
> > basically introduce a second level of overwrites. It might be simpler to
> > not introduce this hierarchy with the cost to break backward
> compatibility.
> >
> > For example, config `request.timeout.ms`:
> >
> > User sets `request.timeout.ms=<user-value>`
> > To change it for the main consumer, user also sets
> > `consumer.request.timeout.ms=<consumer-value>`
> >
> > If user only wants to change the config for main consumer, but not for
> > global or restore consumer, user needs to add two more configs:
> >
> > `restore.consumer.request.timeout.ms=<user-value>`
> > and
> > `global.consumer.request.timeout.ms=<user-value>`
> >
> > to reset both back to the default config. IMHO, this is not an optimal
> > user experience. Thus, it might be worth to change the semantics for
> > `consumer.` prefix to only apply those configs to the main consumer.
> >
> >
> > Not sure what other think what the better solution is (I am not sure by
> > myself to be honest---just wanted to point it out and discuss the
> > pros/cons for both).
> >
> >
> > Another though would be, to introduce one more prefix `main.consumer.`
> > -- using this, the existing `consumer.` prefix would apply to all
> > consumers (keeping it's current semantics) while we have overwrites for
> > all three consumers -- this allow to directly set `main.consumer`
> > instead of `consumer` avoiding the weird pattern from my example above
> > and preserves backward compatibility. Ie, if we introduce an hierarchy
> > of overwrite, a "full" hierarchy might be better than a "partial"
> > hierarchy.
> >
> >
> > Looking forward to your thoughts.
> >
> >
> > -Matthias
> >
> >
> > On 4/1/18 5:55 PM, Guozhang Wang wrote:
> > > Thanks for the KIP Boyang, the KIP looks good to me.
> > >
> > > For config values, we use underscore for keeping a single word; for
> > config
> > > keys, though, we do not use underscores or dashes. I'd suggest using
> dots
> > > to be consistent with others.
> > >
> > >
> > > Otherwise I'm +1 on the KIP.
> > >
> > >
> > > Guozhang
> > >
> > >
> > > On Sun, Apr 1, 2018 at 10:56 AM, Ted Yu <yuzhihong@gmail.com> wrote:
> > >
> > >> Looks good overall.
> > >>
> > >> public static final String RESTORE_CONSUMER_PREFIX =
> > "restore-consumer.";
> > >>
> > >> For other constants in StreamsConfig, underscore is used instead of
> > dash.
> > >>
> > >> Cheers
> > >>
> > >> On Sun, Apr 1, 2018 at 9:38 AM, Boyang Chen <bchen11@outlook.com>
> > wrote:
> > >>
> > >>> Hey friends,
> > >>>
> > >>>
> > >>> I would like to start a discussion thread for KIP 276:
> > >>>
> > >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >>> 276+Add+StreamsConfig+prefix+for+different+consumers
> > >>>
> > >>>
> > >>> And pull request is here:
> > >>>
> > >>> https://github.com/apache/kafka/pull/4805
> > >>>
> > >>> [https://avatars3.githubusercontent.com/u/5845561?s=400&v=4
> ]<https://
> > >>> github.com/apache/kafka/pull/4805>
> > >>>
> > >>> KAFKA-6657: Add StreamsConfig prefix for different consumers by
> > abbccdda
> > >> ·
> > >>> Pull Request #4805 · apache/kafka<https://github.
> > >>> com/apache/kafka/pull/4805>
> > >>> github.com
> > >>> This pull request is for jira 6657. The KIP proposal is here Added
> unit
> > >>> tests for new getGlobalConsumerConfigs API and make sure existing
> > restore
> > >>> consumer tests are passing.
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>> Thanks,
> > >>>
> > >>> Boyang
> > >>>
> > >>
> > >
> > >
> > >
> >
> >
>



-- 
-- Guozhang

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message