jiajunwang commented on a change in pull request #1489:
URL: https://github.com/apache/helix/pull/1489#discussion_r513810797
##########
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 checkForNoOpMessage(Message message, String instanceName,
Review comment:
Any suggestion regarding the name? I feel "checkAndProcessNoOpMessage"
is too long. But maybe that's what we need.
Regarding your 2nd comment, could you please point out the potential code
that throws Exception? I will take care of that in the following PR. We won't
change any business logic in this one.
----------------------------------------------------------------
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]
