-
Notifications
You must be signed in to change notification settings - Fork 3.6k
=clut #24311 timer event may arrive after we already identified successfuly #24442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, no more testing needed for this case
It's only about discarding the unhandled message.
We could use the new timers, but no reason for making big changes.
Yeap, agreed, trying to keep change at a minimum here, at least for now |
Perhaps add the NoSerializationVerficationNeeded to |
Yup, added |
After reading the ticket again, this actually fixes one somewhat more annoying thing than unhandled: "this matches case msg: Any ⇒ so ClusterSingletonProxy will try to send TryToIdentifySingleton to the singleton" That is also harmless, apart from that it would actually try to serialize the TryToIdentifySingleton when sending as a real remote message. |
Yeah, it guards from it to fall through to that case. Added the no serialization check anyway, good to mark it such. |
Test PASSed. |
…uccessfuly (akka#24442) * =clut akka#24311 timer event may arrive after we already identified successfuly * mark as NoSerializationVerificationNeeded
Resolves #24311
I think it is indeed right to accept the message and simply ignore it, like Johan has suggested in the ticket.
Could not think of an useful way of testing it, should I try harder?