echauchot edited a comment on pull request #13061:
URL: https://github.com/apache/beam/pull/13061#issuecomment-722451553
@lukecwik I don't see why this change is necessary because of 2 reasons:
1. all the validates runner tests including multiple window (eg. sliding
windows) already passed.
2. when I wrote this code, I already took some safety mesures about the
modification of the (first only) accumulator during the
`combineFn.mergeAccumulator` by creating a new first accumulator for each
merged window see initial code below:
```
// merge the accumulators for each mergedWindow
...
for (Map.Entry<W, List<Tuple2<AccumT, Instant>>> entry :
mergedWindowToAccumulators.entrySet()) {
...
// we need to create the first accumulator because
combineFn.mergerAccumulators can modify the
// first accumulator
AccumT first = combineFn.createAccumulator();
Iterable<AccumT> accumulatorsToMerge =
Iterables.concat(
Collections.singleton(first),
accumsAndInstantsForMergedWindow.stream()
.map(x -> x._1())
.collect(Collectors.toList()));
...
combineFn.mergeAccumulators(accumulatorsToMerge),
...
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
