[GitHub] [helix] jiajunwang commented on a change in pull request #974: Enforce result check for data accessors batch get calls to prevent partial batch read.

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

jiajunwang commented on a change in pull request #974:
URL: https://github.com/apache/helix/pull/974#discussion_r416941686



##########
File path: helix-core/src/main/java/org/apache/helix/messaging/DefaultMessagingService.java
##########
@@ -186,7 +186,8 @@ private HelixDataAccessor getRecipientDataAccessor(final Criteria recipientCrite
       Map<String, String> sessionIdMap = new HashMap<String, String>();
       if (recipientCriteria.isSessionSpecific()) {
         Builder keyBuilder = targetDataAccessor.keyBuilder();
-        List<LiveInstance> liveInstances = targetDataAccessor.getChildValues(keyBuilder.liveInstances());
+        List<LiveInstance> liveInstances =
+            targetDataAccessor.getChildValues(keyBuilder.liveInstances(), true);

Review comment:
       Retry is in https://github.com/apache/helix/pull/970.
   I thought about this. If the liveInstance list is not complete, I guess we will only send partial messages, I guess it would be bad since there might be some relationship between the messages if they are done within the same pipeline.
   
   But I don't have a strong preference. After PR 970 is in, there is very little chance we see this error. Please let me know if you think this shall be false (and why). I can change it and comment with the reason.




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