Skip to content

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Jan 30, 2018

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?

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Jan 30, 2018
@akka-ci
Copy link

akka-ci commented Jan 30, 2018

Test PASSed.

Copy link
Contributor

@patriknw patriknw left a 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.

@ktoso
Copy link
Contributor Author

ktoso commented Jan 30, 2018

Yeap, agreed, trying to keep change at a minimum here, at least for now

@patriknw
Copy link
Contributor

Perhaps add the NoSerializationVerficationNeeded to TryToIdentifySingleton, since that was one of the things mentioned in the ticket.

@ktoso
Copy link
Contributor Author

ktoso commented Jan 30, 2018

Yup, added

@patriknw
Copy link
Contributor

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.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins and removed tested PR that was successfully built and tested by Jenkins labels Jan 30, 2018
@ktoso
Copy link
Contributor Author

ktoso commented Jan 30, 2018

Yeah, it guards from it to fall through to that case. Added the no serialization check anyway, good to mark it such.

@ktoso ktoso merged commit c7f8429 into akka:master Jan 30, 2018
@ktoso ktoso deleted the wip-singleton-timer-fix branch January 30, 2018 09:17
@akka-ci akka-ci added tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Jan 30, 2018
@akka-ci
Copy link

akka-ci commented Jan 30, 2018

Test PASSed.

manonthegithub pushed a commit to manonthegithub/akka that referenced this pull request Jan 31, 2018
…uccessfuly (akka#24442)

* =clut akka#24311 timer event may arrive after we already identified successfuly

* mark as NoSerializationVerificationNeeded
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants