Fix an inconstant ToString on ConsistentRoutee when the node is remot… #3164
Conversation
|
This bug is pretty nasty and is affecting us when we have a router that includes a local routee with Consistent hashing. Is there a way we can get this one fast tracked into a release? |
|
@annymsMthd oh wow.. that IS nasty. Might have one bit of feedback though. |
| var a2 = new Address("akka.tcp", "RemoteConsistentHashingRouterSpec-1", "client2", 2552); | ||
| var localActor = Sys.ActorOf(Props.Empty, "a"); | ||
|
|
||
| var s1 = new ActorRefRoutee(localActor); |
There was a problem hiding this comment.
should add a comment here / something that makes it obvious how this reproduces the bug
There was a problem hiding this comment.
added it on the method
| .Select(k => consistentHash2.NodeFor(k)) | ||
| .Select(routee => routee.ToString()); | ||
|
|
||
| Assert.Equal(result2, result1); |
There was a problem hiding this comment.
This does a .SequenceEqual call under the covers?
There was a problem hiding this comment.
I think so. I can do a FluentAssertion here that's much clearer.
| @@ -292,7 +292,7 @@ public override string ToString() | |||
| case ActorRefRoutee actorRef: | |||
| return ToStringWithFullAddress(actorRef.Actor.Path); | |||
| case ActorSelectionRoutee selection: | |||
| return ToStringWithFullAddress(selection.Selection.Anchor.Path) + | |||
| return ToStringWithFullAddress(selection.Selection.Anchor.Path).TrimEnd('/') + | |||
There was a problem hiding this comment.
So it was just the trailing slash that created this issue?
There was a problem hiding this comment.
Yup. It results in double slashes. When everything is remote it works because they all have it. If you have a local routee you start seeing a little weirdness.
|
@annymsMthd does this need to be tested on clustered consistent hash routers also? Or is the test case here sufficient? |
|
Since the Clustered router uses the ConsistentHash in it's internals I think it should be fine just testing it at this level. I'm on a bus but once I get into the office I'll update this PR with your suggestions. |
…e vs local Signed-off-by: Joshua Benjamin <benjamin@syncromatics.com>
…e vs local
Signed-off-by: Joshua Benjamin benjamin@syncromatics.com