jiajunwang commented on a change in pull request #1124:
URL: https://github.com/apache/helix/pull/1124#discussion_r454660454
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java
##########
@@ -344,6 +367,13 @@ private void schedulePendingMessageCleanUp(
if (accessor.removeProperty(msg.getKey(accessor.keyBuilder(),
instanceName))) {
LogUtil.logInfo(logger, _eventId, String
.format("Deleted message %s from instance %s",
msg.getMsgId(), instanceName));
+ if (staleMessageMap != null &&
staleMessageMap.containsKey(msg.getTgtName())
+ &&
staleMessageMap.get(msg.getTgtName()).containsKey(msg.getMsgId())) {
Review comment:
Both versions are safe. Just too verbose.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java
##########
@@ -371,6 +401,14 @@ private boolean shouldCleanUpPendingMessage(Message
pendingMsg, String currentSt
}
}
+ private boolean shouldCleanUpStaleMessage(Message staleMsg, Long
currentStateTransitionEndTime) {
+ if (staleMsg == null) {
Review comment:
I personally prefer to fail early. If it should not, then we should not
check. The concern is that overprotected code will hide potential issues.
For example, if some bugs make it always null, then you will always get
false as a result. But it would be better that we get an NPE, then the bug will
be identified the first time.
----------------------------------------------------------------
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]
