lukecwik commented on a change in pull request #12994:
URL: https://github.com/apache/beam/pull/12994#discussion_r501855713
##########
File path:
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/GroupingShuffleReader.java
##########
@@ -402,6 +432,14 @@ public boolean hasNext() {
@Override
public V next() {
+ // Given that the underlying ReadOperation already checks the abort
status after every
+ // record it advances over (i.e., for every distinct key), we skip the
check when at
+ // the first value as that is redundant. Signal by thread interruption
may be better, but
+ // it may also have unintended side-effects.
+ if (!atFirstValue && aborted.get()) {
Review comment:
Checking atFirstValue and aborted will likely perform worse then just
checking aborted all the time. It may seem redundant but the abort happens
asynchronously so we may have gotten past the check in the ReadOperation
already.
----------------------------------------------------------------
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]
