Github user mbalassi commented on the pull request:
https://github.com/apache/flink/pull/1069#issuecomment-135418178
Thanks for the fast response on your side addressing my comments. :)
I generally love the idea described at
[FLINK-1725](https://issues.apache.org/jira/browse/FLINK-1725), I have a couple
of concerns though:
1. Having the code in is nice, but we need a way to surface it in the
`DataStream` API. The current implementation can not do that in a
straight-forward way, as you need the number of output channels in a
constructor parameter. That is an information that we do not have when creating
the partitioning strategy for the data stream yet (E.g.
[here](https://github.com/apache/flink/blob/master/flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/datastream/DataStream.java#L502-L504)).
2. The comment you have added to `PartialPartitioner` did not give me a
clear picture of the behaviour. I much prefer the one you wrote
[here](https://issues.apache.org/jira/browse/FLINK-1725?focusedCommentId=14371032&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14371032).
Some comment would be nice on the test side too, I only got it after reading
the aforementioned JIRA issue.
3. How sensitive are your algorithms fed by this partitioning to loosing
the state in `targetChannelTasks`? Because if a Flink operator was to go down
this information would not be recovered after failure.
4. I share the wish of @gdfm that it would be really nice if the
implementation did not depend on `toString`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
