NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504128305
##########
File path:
helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws
InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" +
_correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" +
_correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out
already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
You mean they wouldn't be able to distinguish "stale messages" vs
"messages that don't have callbacks set up correctly"? I think these cases are
logically the same and can't be easily distinguished code-level. If we are
loosening up on one case, it's going to affect the other.
On the other hand, callback registration is done in our code and naturally
there should be a callback before sending messages that require replies. I
imagine the previous check was there for cautionary purpose, not that messages
are commonly created with incorrect correlation ids.
----------------------------------------------------------------
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]
