tillrohrmann commented on a change in pull request #14940:
URL: https://github.com/apache/flink/pull/14940#discussion_r576170084
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/FutureUtils.java
##########
@@ -808,6 +808,23 @@ private void handleCompletedFuture(int index, T value,
Throwable throwable) {
if (throwable != null) {
completeExceptionally(throwable);
} else {
+ /**
+ * This {@link #results} update itself is not synchronised in
any way and it's fine
+ * because:
+ *
+ * <ul>
+ * <li>There is a happens-before relationship for each
thread (that is completing
+ * the future) between setting {@link #results} and
incrementing {@link
+ * #numCompleted}.
+ * <li>Each thread is updating uniquely different field of
the {@link #results}
+ * array.
+ * <li>There is a happens-before relationship between all of
the writing threads
+ * and the last one thread.
+ * <li>The last thread will be completing the future, so it
has transitively
+ * happens-before relationship with all of preceding
updated/writes to {@link
+ * #results}.
Review comment:
Maybe add why the happens-before relationship is there
(`AtomicInteger.incrementAndGet` which is a volatile read & write).
----------------------------------------------------------------
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]
