Re: [DISCUSS]refactor StreamConfig

2017-07-07 Thread Aljoscha Krettek
Hi, Yes, that sounds very good! I like the idea of having an interface that is a view on some of the settings. We could maybe improve the naming, because to me it is not completely clear what the difference between OperatorSettings and OperatorProperties is (I mean just the name, the functional

Re:[DISCUSS]refactor StreamConfig

2017-07-06 Thread Xu Pingyong
Hi Aljoscha: Great. I cannot agree with you more. So I introduce OperatorSettings and OperatorProperties. StreamTaskConfig relys on the underlying configuration and is provided for the streamTask to use. It contains: 1) in.physical.edges 2) out.physical.edges 3) chained

Re: refactor StreamConfig

2017-07-06 Thread Aljoscha Krettek
Hi, Yes, the fact that the operator can see isChainStart() and isChainEnd() is not good, in my opinion. These seems to be an implementation detail that an operator should not be aware of. For now it’s ok but maybe we can fix that later. Regarding output edges and serialisers, I think it might

Re: refactor StreamConfig

2017-07-04 Thread Xu Pingyong
Hi Aljoscha: Ye, I agree with you that an operator should not see output edges and serialisers. The call getChainIndex() is used only in OperatorConfig.toString(), it can be removed. However, isChainStart() and isChainEnd() is used in AbstractStreamOperator.setup(...). But I think what Steph

Re: refactor StreamConfig

2017-07-04 Thread Aljoscha Krettek
Hi, Yes, but I think what Stephan was hinting at was to change both of them to be serialisable when already working on this. I think input serialiser is fine to have in OperatorConfig, you’re right! I don’t see getChainIndex() used anywhere in the code, though. And the output edges and seriali

Re:Re: refactor StreamConfig

2017-07-04 Thread xu
Hi Aljoscha: Thanks a lot for your advice. I think I have not need to separate steps, because what I do is only that introducing OperatorConfig and moving the fields. StreamConfig still relys on an underlying Configuration which flows from client to the jobmanager and then to the tas

Re: refactor StreamConfig (Appending a picture)

2017-07-04 Thread Ted Yu
The picture didn't go thru. Please use third party site. On Tue, Jul 4, 2017 at 7:09 AM, xu wrote: > I All: > I am sorry about working with StreamConfig(https://github. > com/apache/flink/pull/4241) which may conflicts with others' work before > discussing. > > Motivation: >

Re: refactor StreamConfig

2017-07-04 Thread Aljoscha Krettek
I think the proposed changed are good, I just wanted to make sure that they don’t interfere with what other people are doing. I also proposed these steps on the Github PR: Also, for actually doing the changes I suggest separate steps, i.e. separate commits. With possibly separate PRs to make rev

refactor StreamConfig

2017-07-04 Thread xu
HI All: I am sorry about working with StreamConfig(https://github.com/apache/flink/pull/4241) which may conflicts with others' work before discussing. Motivation: A Task contains one or more operators with chainning, however configs of operator and task are all put in Stream