lukecwik commented on a change in pull request #16769:
URL: https://github.com/apache/beam/pull/16769#discussion_r801296671
##########
File path:
sdks/java/fn-execution/src/main/java/org/apache/beam/sdk/fn/data/BeamFnDataOutboundAggregator.java
##########
@@ -199,7 +210,14 @@ public void sendBufferedDataAndFinishOutboundStreams() {
.setIsLast(true);
entry.getValue().resetStats();
}
+ if (collectElementsIfNoFlushes && !hasFlushedForBundle) {
+ return bufferedElements.build();
+ }
outboundObserver.onNext(bufferedElements.build());
+ // This is now at the end of a bundle, so we reset hasFlushedForBundle to
prepare for new
+ // bundles.
+ hasFlushedForBundle = false;
Review comment:
I see now what you're saying. Can you add `@NotThreadSafe` annotation at
the class level and add this comment above it:
The calling thread that invokes `sendBufferedDataAndFinishOutboundStreams`
synchronizes on `flushLock` effectively making the periodic flushing no longer
read or mutate `hasFlushedForBundle` and allowing the calling thread to read
and mutate `hasFlushedForBundle` safely without needing to create another
memory barrier. Also note that `flush` is always invoked when synchronizing on
`flushLock` when there is a periodic flushing thread.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
