kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boyang Chen <bche...@outlook.com>
Subject Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different consumers
Date Tue, 03 Apr 2018 05:14:12 GMT
Hey friends,


both KIP<https://cwiki.apache.org/confluence/display/KAFKA/KIP-276+Add+StreamsConfig+prefix+for+different+consumers>
and pull request<https://github.com/apache/kafka/pull/4805> are updated. Feel free to
take another look.



Thanks for your valuable feedback!


Best,

Boyang

________________________________
From: Boyang Chen <bchen11@outlook.com>
Sent: Tuesday, April 3, 2018 11:39 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different consumers

Thanks Matthias, Ted and Guozhang for the inputs. I shall address them in next round.


________________________________
From: Matthias J. Sax <matthias@confluent.io>
Sent: Tuesday, April 3, 2018 4:43 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different consumers

Yes, your examples make sense to me. That was the idea behind the proposal.


-Matthias

On 4/2/18 11:25 AM, Guozhang Wang wrote:
> @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
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
>


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