[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-28 Thread zhijiangW
Github user zhijiangW commented on the issue: https://github.com/apache/flink/pull/3303 @StephanEwen , sorry for my carelessness of **checkNotNull**, it is a low mistake. And I passed the "clean verify" in my local machine, thank you for merging! --- If your project is set up for

[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-27 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3303 I have merged this to my local repository. There were some issues left, partly in the commented out code. In particular `checkNotNull(variable !=null) does not work, because

[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-27 Thread zhijiangW
Github user zhijiangW commented on the issue: https://github.com/apache/flink/pull/3303 Hi @StephanEwen , thanks for detail reviews of this PR and I learnt a lot from your comments. I considered all your suggestions above and submitted the modifications, including:

[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-24 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3303 One thought that @tillrohrmann and me had: It is probably okay to comment out or remove the **setters** and keep the **getters**. That should help in keeping the internal code. --- If your

[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-24 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3303 A more general question on the resource matching: If I understand it correctly, then the resource manager will try to get the "max" resources for an operator, but potentially go down to the

[GitHub] flink issue #3303: [FLINK-5133][core] Support to set resource for operator i...

2017-02-24 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3303 The code here looks very good, with a few minor comments. The main problem is as you mentioned: We are adding something to the API that is not yet supported by the runtime. We have done