kaisun2000 commented on a change in pull request #1489:
URL: https://github.com/apache/helix/pull/1489#discussion_r514547303
##########
File path:
helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java
##########
@@ -997,30 +873,186 @@ public void onMessage(String instanceName, List<Message>
messages,
}
}
- // update message state to READ in batch and schedule all read messages
+ // update message state to READ in batch and schedule tasks for all read
messages
if (readMsgs.size() > 0) {
updateMessageState(readMsgs, accessor, instanceName);
- // Remove message if schedule tasks are failed.
for (Map.Entry<String, MessageHandler> handlerEntry :
stateTransitionHandlers.entrySet()) {
MessageHandler handler = handlerEntry.getValue();
NotificationContext context =
stateTransitionContexts.get(handlerEntry.getKey());
- Message msg = handler._message;
- if (!scheduleTask(new HelixTask(msg, context, handler, this))) {
- removeMessageFromTaskAndFutureMap(msg);
- removeMessageFromZK(accessor, msg, instanceName);
- }
+ scheduleTaskForMessage(instanceName, accessor, handler, context);
}
for (int i = 0; i < nonStateTransitionHandlers.size(); i++) {
MessageHandler handler = nonStateTransitionHandlers.get(i);
NotificationContext context = nonStateTransitionContexts.get(i);
- Message msg = handler._message;
- if (!scheduleTask(new HelixTask(msg, context, handler, this))) {
- removeMessageFromTaskAndFutureMap(msg);
- removeMessageFromZK(accessor, msg, instanceName);
+ scheduleTaskForMessage(instanceName, accessor, handler, context);
+ }
+ }
+ }
+
+ /**
+ * Inspect the message. Report and remove it if no operation needs to be
done.
+ * @param message
+ * @param instanceName
+ * @param changeContext
+ * @param manager
+ * @param sessionId
+ * @param stateTransitionHandlers
+ * @return True if the message is no-op message and no other process step is
required.
+ */
+ private boolean checkAndProcessNoOpMessage(Message message, String
instanceName,
+ NotificationContext changeContext, HelixManager manager, String
sessionId,
+ Map<String, MessageHandler> stateTransitionHandlers) {
+ HelixDataAccessor accessor = manager.getHelixDataAccessor();
Review comment:
```
public HelixDataAccessor getHelixDataAccessor() {
checkConnected(_waitForConnectedTimeout);
return _dataAccessor;
}
```
if somehow the _zkClient is closed, the above code would throw exception,
which would cause the same issue of repeatedly controller sending messages.
Note, with new change to statusUpdateUtil, we won't really use _zkClient
from ZKHelixManager to write to Zookeeper.
In sum, I think the core idea (invariant) is that don't throw anything out
from this method. Anyway, up to you.
----------------------------------------------------------------
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]
