[GitHub] [helix] lei-xia commented on a change in pull request #1002: Use updaters to update read messages to ZK

[GitBox <gi...@apache.org>]

lei-xia commented on a change in pull request #1002:
URL: https://github.com/apache/helix/pull/1002#discussion_r426793803



##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java
##########
@@ -524,12 +527,27 @@ public void finishTask(MessageTask task) {
   private void updateMessageState(List<Message> readMsgs, HelixDataAccessor accessor,
       String instanceName) {
     Builder keyBuilder = accessor.keyBuilder();
-    List<PropertyKey> readMsgKeys = new ArrayList<>();
+    List<String> readMsgPaths = new ArrayList<>();
+    List<DataUpdater<ZNRecord>> updaters = new ArrayList<>();
     for (Message msg : readMsgs) {
-      readMsgKeys.add(msg.getKey(keyBuilder, instanceName));
+      readMsgPaths.add(msg.getKey(keyBuilder, instanceName).getPath());
       _knownMessageIds.add(msg.getId());
+      /**
+       * We use the updater to avoid race condition between writing message to zk as READ state and removing message after ST is done
+       * If there is no message at this path, meaning the message is removed so we do not write the message
+       */
+      updaters.add(new DataUpdater<ZNRecord>() {

Review comment:
       One optimization here maybe to create just one updater instead of one per each message. You can define the update logic in the updater, i.e, read the message and mark the read field as "read".  




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org