[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-05-12 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/370 --- 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 enabl

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-19 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-75030486 okay, I will the remove the chained combiner. I'm curious though, the issue description specifically mentions the AllGroupReduce greatly benefiting from a chained v

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-16 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74565386 I think we should use the chained reducer from this pull request and drop the chained combiner change. --- If your project is set up for it, you can reply to this ema

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74247815 Then it is worse then the chained variant, because you save nothing and need buffer space. The chained variant forwards records/fragments as they are produces. --- I

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74247290 i know, so could we not do the same, except skipping the sorting part. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHu

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74247098 The synchronous combine driver stores them in the sort buffer in serialized fashion, which is save. --- If your project is set up for it, you can reply to this email

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74240805 the SynchronouesChainedCombineDriver stores multiple records as well doesn't it? except up to a fixed size instead of # of elements. could we not do the same here? --- If

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74236667 The chained reducer variant makes more sense. The question is, what is cheaper - serialized exchange through the in-mem-channel, or the copying. My gut feeling is that

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/370#issuecomment-74236482 I think for the combiner, we cannot do this. The way it is done here easily blows up the memory, by collecting so many records. Bear in mind that a record is not neces

[GitHub] flink pull request: [FLINK-785] Chained AllReduce / AllGroupReduce...

2015-02-12 Thread zentol
Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/370#discussion_r24633778 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/operators/chaining/ChainedAllGroupReduceCombineDriver.java --- @@ -0,0 +1,118 @@ +/* + *