jiajunwang commented on a change in pull request #1124:
URL: https://github.com/apache/helix/pull/1124#discussion_r458988730
##########
File path:
helix-core/src/main/java/org/apache/helix/common/caches/InstanceMessagesCache.java
##########
@@ -148,9 +152,20 @@ public boolean refresh(HelixDataAccessor accessor,
Map<String, LiveInstance> liv
LOG.info(
"END: InstanceMessagesCache.refresh(), {} of Messages read from
ZooKeeper. took {} ms. ",
newMessageKeys.size(), (System.currentTimeMillis() - startTime));
+
+ refreshStaleMessageCache();
return true;
}
+ public Map<String, Map<String, Message>> getStaleMessageCache() {
Review comment:
I think we have this method mainly for supporting the test. Otherwise,
we can have the getStaleMessageForInstance() here, right?
My suggestion is that we should not change our business logic in any way
because of the test requirement. So I will say, let's move the
getStaleMessageForInstance() implementation here. And leave this method as
@VisibleForTesting only.
##########
File path:
helix-core/src/main/java/org/apache/helix/common/caches/InstanceMessagesCache.java
##########
@@ -148,9 +152,20 @@ public boolean refresh(HelixDataAccessor accessor,
Map<String, LiveInstance> liv
LOG.info(
"END: InstanceMessagesCache.refresh(), {} of Messages read from
ZooKeeper. took {} ms. ",
newMessageKeys.size(), (System.currentTimeMillis() - startTime));
+
+ refreshStaleMessageCache();
return true;
}
+ public Map<String, Map<String, Message>> getStaleMessageCache() {
Review comment:
Or, if you can make the test to check InstanceMessagesCache only, that
would be even better. But that is optional.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/dataproviders/BaseControllerDataProvider.java
##########
@@ -544,6 +544,27 @@ public synchronized void
setLiveInstances(List<LiveInstance> liveInstances) {
return _instanceMessagesCache.getMessages(instanceName);
}
+ /**
+ * This function is supposed to be only used by testing purpose for safety.
For "get" usage,
+ * please use getStaleMessagesByInstance.
+ */
+ public Map<String, Map<String, Message>> getStaleMessages() {
Review comment:
Let's add a @VisibleForTesting tag here
----------------------------------------------------------------
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]
