pkuwm commented on a change in pull request #1066:
URL: https://github.com/apache/helix/pull/1066#discussion_r436228222



##########
File path: 
helix-core/src/main/java/org/apache/helix/controller/stages/MessageDispatchStage.java
##########
@@ -78,7 +78,13 @@ protected void processEvent(ClusterEvent event, 
MessageOutput messageOutput) thr
         batchMessage(dataAccessor.keyBuilder(), messagesToSend, resourceMap, 
liveInstanceMap,
             manager.getProperties());
 
-    List<Message> messagesSent = sendMessages(dataAccessor, outputMessages);
+    String expectedSession = 
event.getAttribute(AttributeName.CONTROLLER_LEADER_SESSION.name());
+    // An early check for expected leader session. If the sessions don't 
match, it means the
+    // controller lost leadership, then messages should not be sent. This 
potentially avoid
+    // double masters for a single partition.
+    List<Message> messagesSent = manager.getSessionId().equals(expectedSession)

Review comment:
       I proposed to do some checks at stages like `ReadClusterDataStage` and 
terminate the remaining pipeline if controller loses leadership. In our issue, 
I see that leader lost leadership at `ReadClusterDataStage`. So there is no 
need to continue the pipeline and it would help save some computation resource 
or write traffic to EV/persist assignment. But in the discussion we agreed on 
not doing that and just blocking messages. We could definitely pick it up and 
discuss again. @jiajunwang 




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to