StephanEwen commented on a change in pull request #15557:
URL: https://github.com/apache/flink/pull/15557#discussion_r613165874



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/operators/coordination/OperatorEventValve.java
##########
@@ -96,42 +94,44 @@ public boolean isShut() {
      * (because it gets dropped through a call to {@link #reset()} or {@link 
#resetForTask(int)},
      * then the returned future till be completed exceptionally.
      */
-    public CompletableFuture<Acknowledge> sendEvent(
-            SerializedValue<OperatorEvent> event, int subtask) {
-        synchronized (lock) {
-            if (!shut) {
-                return eventSender.apply(event, subtask);
-            }
-
-            final List<BlockedEvent> eventsForTask =
-                    blockedEvents.computeIfAbsent(subtask, (key) -> new 
ArrayList<>());
-            final CompletableFuture<Acknowledge> future = new 
CompletableFuture<>();
-            eventsForTask.add(new BlockedEvent(event, subtask, future));
-            return future;
+    public void sendEvent(
+            SerializedValue<OperatorEvent> event,
+            int subtask,
+            CompletableFuture<Acknowledge> result) {

Review comment:
       I dropped doing parameter JavaDocs for most cases, where the parameters 
should be intuitive. I found that the parameter docs often add more noise then 
benefit.
   
   I would add a note about the contract for the future to the method's 
JavaDocs. The contract is: Doesn't matter who completes the future, as long as 
it is eventually completed (successfully or exceptionally).




-- 
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]


Reply via email to