dasahcc commented on a change in pull request #1066:
URL: https://github.com/apache/helix/pull/1066#discussion_r437617247
##########
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 think stopping the pipeline is much better. Because that saves you
time if client reconnect with different session for next pipeline.
I am OK to skip the change in the PR. But please add it to the TODO part.
This is necessary especially when our pipeline takes much longer time for
larger amount of resources computation.
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManager.java
##########
@@ -425,6 +426,20 @@ void
addExternalViewChangeListener(org.apache.helix.ExternalViewChangeListener l
*/
boolean isLeader();
+ /**
+ * Checks whether the cluster manager is leader and sets its ZK session in
param
+ * {@link InstanceLeaderSession} if and only if it is leader.
+ *
+ * @param instanceLeaderSession To include ZK session ID of the cluster
manager in return
+ *
+ * @return true if the instance is a leader of the cluster and Zk session of
the cluster
+ * manager is return in param {@link InstanceLeaderSession}
+ */
+ default boolean isInstanceLeader(InstanceLeaderSession
instanceLeaderSession) {
+ throw new UnsupportedOperationException(
+ "Checking leader and returning session is not supported.");
+ }
Review comment:
Let's make the naming consistent. Better to be isLeader(Session).
----------------------------------------------------------------
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]
