Re: Stream config in StreamPartitionAssignor

2018-07-10 Thread Boyang Chen
Sounds good. I will take a look at the doc and get back to you!



From: Guozhang Wang 
Sent: Wednesday, July 11, 2018 2:07 AM
To: dev
Subject: Re: Stream config in StreamPartitionAssignor

Boyang,

I see. That makes sense. We should at least update the docs to reduce
confusions.


Guozhang


On Mon, Jul 9, 2018 at 10:22 PM, Boyang Chen  wrote:

> Hey Guozhang,
>
>
> thanks for the reply. Actually I'm not trying to pass config into
> StreamPartitionAssignor. My debugging was about some other configs that
> wasn't transferred properly (completely irrelevant), but the effort was
> delayed because the log would print "StreamsConfig values" for
> StreamPartitionAssignor, where it will fallback all the config to defaults.
> So it would give me as an end user the impression that my config passed in
> was "not successfully" conveyed to each Stream Thread.
>
>
> I'm thinking whether we could disable stream partitioner to print out its
> config in the log, or have a more explicit way saying this is not Stream
> Thread config? Hope I made myself clear to you.
>
>
> Also yes, I could put some comment in the docs to let user use Consumer
> config prefix if that could help.
>
>
> Best,
>
> Boyang
>
> ________
> From: Guozhang Wang 
> Sent: Tuesday, July 10, 2018 5:35 AM
> To: dev
> Subject: Re: Stream config in StreamPartitionAssignor
>
> Hello Boyang,
>
> Thanks for brining this up. Currently since the StreamsPartitionAssingor
> can only be initiated within the Consumer instance, I think letting users
> to pass in the config values with prefix is the preferrable way, i.e. we
> can improve our docs to educate users about that. BTW I'm curious what
> configs you want to pass into the StreamsPartitionAssignor? Currently since
> there is no user configurable part inside that class, I do not know what
> configs can take effects.
>
>
> Guozhang
>
> On Mon, Jul 9, 2018 at 2:06 PM, Boyang Chen  wrote:
>
> > Hey there,
> >
> >
> > over the weekend I was debugging the streams configuration not passed
> > within threads. I noticed that one of the code path from KafkaConsumer
> > (L743) was to initialize the StreamPartitionAssignor:
> >
> > this.assignors = config.getConfiguredInstances(
> > ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
> > PartitionAssignor.class);
> >
> > However, it was using the ConsumerConfig instance (that config is passed
> > in), so if I want to make some configuration change in the assignor, I
> need
> > to put consumer prefix. To make the debugging even harder, there was an
> > logAll() function in AbstractConfig which will print "StreamsConfig
> values"
> > at the beginning, since it is indeed a stream config:
> >
> > @Override
> > public void configure(final Map configs) {
> > final StreamsConfig streamsConfig = new StreamsConfig(configs);
> >
> > (L190 in StreamPartitionAssignor)
> >
> >
> > This would further confuse developer as they see two different sets of
> > StreamsConfig: one from top level, one from this derived level per
> thread.
> >
> >
> > My point is that we could either: 1. let developer be aware that they
> need
> > to add consumer prefix to pass in configs to StreamPartitionAssignor 2.
> we
> > found a way to pass in original StreamsConfig.
> >
> > I know this is a little bit lengthy description, let me know if you feel
> > unclear about my proposal, or this is not a concern since most people
> > already know the trick here, thank you!
> >
> >
>
>
> --
> -- Guozhang
>



--
-- Guozhang


Re: Stream config in StreamPartitionAssignor

2018-07-10 Thread Guozhang Wang
Boyang,

I see. That makes sense. We should at least update the docs to reduce
confusions.


Guozhang


On Mon, Jul 9, 2018 at 10:22 PM, Boyang Chen  wrote:

> Hey Guozhang,
>
>
> thanks for the reply. Actually I'm not trying to pass config into
> StreamPartitionAssignor. My debugging was about some other configs that
> wasn't transferred properly (completely irrelevant), but the effort was
> delayed because the log would print "StreamsConfig values" for
> StreamPartitionAssignor, where it will fallback all the config to defaults.
> So it would give me as an end user the impression that my config passed in
> was "not successfully" conveyed to each Stream Thread.
>
>
> I'm thinking whether we could disable stream partitioner to print out its
> config in the log, or have a more explicit way saying this is not Stream
> Thread config? Hope I made myself clear to you.
>
>
> Also yes, I could put some comment in the docs to let user use Consumer
> config prefix if that could help.
>
>
> Best,
>
> Boyang
>
> ________
> From: Guozhang Wang 
> Sent: Tuesday, July 10, 2018 5:35 AM
> To: dev
> Subject: Re: Stream config in StreamPartitionAssignor
>
> Hello Boyang,
>
> Thanks for brining this up. Currently since the StreamsPartitionAssingor
> can only be initiated within the Consumer instance, I think letting users
> to pass in the config values with prefix is the preferrable way, i.e. we
> can improve our docs to educate users about that. BTW I'm curious what
> configs you want to pass into the StreamsPartitionAssignor? Currently since
> there is no user configurable part inside that class, I do not know what
> configs can take effects.
>
>
> Guozhang
>
> On Mon, Jul 9, 2018 at 2:06 PM, Boyang Chen  wrote:
>
> > Hey there,
> >
> >
> > over the weekend I was debugging the streams configuration not passed
> > within threads. I noticed that one of the code path from KafkaConsumer
> > (L743) was to initialize the StreamPartitionAssignor:
> >
> > this.assignors = config.getConfiguredInstances(
> > ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
> > PartitionAssignor.class);
> >
> > However, it was using the ConsumerConfig instance (that config is passed
> > in), so if I want to make some configuration change in the assignor, I
> need
> > to put consumer prefix. To make the debugging even harder, there was an
> > logAll() function in AbstractConfig which will print "StreamsConfig
> values"
> > at the beginning, since it is indeed a stream config:
> >
> > @Override
> > public void configure(final Map configs) {
> > final StreamsConfig streamsConfig = new StreamsConfig(configs);
> >
> > (L190 in StreamPartitionAssignor)
> >
> >
> > This would further confuse developer as they see two different sets of
> > StreamsConfig: one from top level, one from this derived level per
> thread.
> >
> >
> > My point is that we could either: 1. let developer be aware that they
> need
> > to add consumer prefix to pass in configs to StreamPartitionAssignor 2.
> we
> > found a way to pass in original StreamsConfig.
> >
> > I know this is a little bit lengthy description, let me know if you feel
> > unclear about my proposal, or this is not a concern since most people
> > already know the trick here, thank you!
> >
> >
>
>
> --
> -- Guozhang
>



-- 
-- Guozhang


Re: Stream config in StreamPartitionAssignor

2018-07-09 Thread Boyang Chen
Hey Guozhang,


thanks for the reply. Actually I'm not trying to pass config into 
StreamPartitionAssignor. My debugging was about some other configs that wasn't 
transferred properly (completely irrelevant), but the effort was delayed 
because the log would print "StreamsConfig values" for StreamPartitionAssignor, 
where it will fallback all the config to defaults. So it would give me as an 
end user the impression that my config passed in was "not successfully" 
conveyed to each Stream Thread.


I'm thinking whether we could disable stream partitioner to print out its 
config in the log, or have a more explicit way saying this is not Stream Thread 
config? Hope I made myself clear to you.


Also yes, I could put some comment in the docs to let user use Consumer config 
prefix if that could help.


Best,

Boyang


From: Guozhang Wang 
Sent: Tuesday, July 10, 2018 5:35 AM
To: dev
Subject: Re: Stream config in StreamPartitionAssignor

Hello Boyang,

Thanks for brining this up. Currently since the StreamsPartitionAssingor
can only be initiated within the Consumer instance, I think letting users
to pass in the config values with prefix is the preferrable way, i.e. we
can improve our docs to educate users about that. BTW I'm curious what
configs you want to pass into the StreamsPartitionAssignor? Currently since
there is no user configurable part inside that class, I do not know what
configs can take effects.


Guozhang

On Mon, Jul 9, 2018 at 2:06 PM, Boyang Chen  wrote:

> Hey there,
>
>
> over the weekend I was debugging the streams configuration not passed
> within threads. I noticed that one of the code path from KafkaConsumer
> (L743) was to initialize the StreamPartitionAssignor:
>
> this.assignors = config.getConfiguredInstances(
> ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
> PartitionAssignor.class);
>
> However, it was using the ConsumerConfig instance (that config is passed
> in), so if I want to make some configuration change in the assignor, I need
> to put consumer prefix. To make the debugging even harder, there was an
> logAll() function in AbstractConfig which will print "StreamsConfig values"
> at the beginning, since it is indeed a stream config:
>
> @Override
> public void configure(final Map configs) {
> final StreamsConfig streamsConfig = new StreamsConfig(configs);
>
> (L190 in StreamPartitionAssignor)
>
>
> This would further confuse developer as they see two different sets of
> StreamsConfig: one from top level, one from this derived level per thread.
>
>
> My point is that we could either: 1. let developer be aware that they need
> to add consumer prefix to pass in configs to StreamPartitionAssignor 2. we
> found a way to pass in original StreamsConfig.
>
> I know this is a little bit lengthy description, let me know if you feel
> unclear about my proposal, or this is not a concern since most people
> already know the trick here, thank you!
>
>


--
-- Guozhang


Re: Stream config in StreamPartitionAssignor

2018-07-09 Thread Guozhang Wang
Hello Boyang,

Thanks for brining this up. Currently since the StreamsPartitionAssingor
can only be initiated within the Consumer instance, I think letting users
to pass in the config values with prefix is the preferrable way, i.e. we
can improve our docs to educate users about that. BTW I'm curious what
configs you want to pass into the StreamsPartitionAssignor? Currently since
there is no user configurable part inside that class, I do not know what
configs can take effects.


Guozhang

On Mon, Jul 9, 2018 at 2:06 PM, Boyang Chen  wrote:

> Hey there,
>
>
> over the weekend I was debugging the streams configuration not passed
> within threads. I noticed that one of the code path from KafkaConsumer
> (L743) was to initialize the StreamPartitionAssignor:
>
> this.assignors = config.getConfiguredInstances(
> ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
> PartitionAssignor.class);
>
> However, it was using the ConsumerConfig instance (that config is passed
> in), so if I want to make some configuration change in the assignor, I need
> to put consumer prefix. To make the debugging even harder, there was an
> logAll() function in AbstractConfig which will print "StreamsConfig values"
> at the beginning, since it is indeed a stream config:
>
> @Override
> public void configure(final Map configs) {
> final StreamsConfig streamsConfig = new StreamsConfig(configs);
>
> (L190 in StreamPartitionAssignor)
>
>
> This would further confuse developer as they see two different sets of
> StreamsConfig: one from top level, one from this derived level per thread.
>
>
> My point is that we could either: 1. let developer be aware that they need
> to add consumer prefix to pass in configs to StreamPartitionAssignor 2. we
> found a way to pass in original StreamsConfig.
>
> I know this is a little bit lengthy description, let me know if you feel
> unclear about my proposal, or this is not a concern since most people
> already know the trick here, thank you!
>
>


-- 
-- Guozhang


Stream config in StreamPartitionAssignor

2018-07-09 Thread Boyang Chen
Hey there,


over the weekend I was debugging the streams configuration not passed within 
threads. I noticed that one of the code path from KafkaConsumer (L743) was to 
initialize the StreamPartitionAssignor:

this.assignors = config.getConfiguredInstances(
ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
PartitionAssignor.class);

However, it was using the ConsumerConfig instance (that config is passed in), 
so if I want to make some configuration change in the assignor, I need to put 
consumer prefix. To make the debugging even harder, there was an logAll() 
function in AbstractConfig which will print "StreamsConfig values" at the 
beginning, since it is indeed a stream config:

@Override
public void configure(final Map configs) {
final StreamsConfig streamsConfig = new StreamsConfig(configs);

(L190 in StreamPartitionAssignor)


This would further confuse developer as they see two different sets of 
StreamsConfig: one from top level, one from this derived level per thread.


My point is that we could either: 1. let developer be aware that they need to 
add consumer prefix to pass in configs to StreamPartitionAssignor 2. we found a 
way to pass in original StreamsConfig.

I know this is a little bit lengthy description, let me know if you feel 
unclear about my proposal, or this is not a concern since most people already 
know the trick here, thank you!