Skip to content
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

Fix an inconstant ToString on ConsistentRoutee when the node is remot… #3164

Merged
merged 1 commit into from
Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/core/Akka.Remote.Tests/RemoteConsistentHashingRouterSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,44 @@ public void ConsistentHashingGroup_must_use_same_hash_ring_independent_of_self_a
var result2 = keys.Select(k => consistentHash2.NodeFor(k).Routee);
Assert.Equal(result2,result1);
}

/// <summary>
/// This test creates two nodes each with a local routee to themselves and a remote routee to the other node.
/// When using ToString on ConsistentRoutee the local and remote routees are treated differently.
/// This test ensures the two are hashed the same.
/// </summary>
[Fact]
public void ConsistentHashingGroup_must_use_same_hash_ring_independent_of_local_and_remote_nodes()
{
var a1 = new Address("akka.tcp", "RemoteConsistentHashingRouterSpec-1", "client1", 2552);
var a2 = new Address("akka.tcp", "RemoteConsistentHashingRouterSpec-1", "client2", 2552);
var localActor = Sys.ActorOf(Props.Empty, "a");

var s1 = new ActorRefRoutee(localActor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add a comment here / something that makes it obvious how this reproduces the bug

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added it on the method

var s2 = new ActorSelectionRoutee(Sys.ActorSelection("akka.tcp://RemoteConsistentHashingRouterSpec-1@client2:2552/user/a"));
var nodes1 = new List<ConsistentRoutee>(new[] { new ConsistentRoutee(s1, a1), new ConsistentRoutee(s2, a1) });

var s4 = new ActorSelectionRoutee(Sys.ActorSelection("akka.tcp://RemoteConsistentHashingRouterSpec-1@client1:2552/user/a"));
var s5 = new ActorRefRoutee(localActor);

var nodes2 = new List<ConsistentRoutee>(new[] { new ConsistentRoutee(s5, a2), new ConsistentRoutee(s4, a2) });

var consistentHash1 = ConsistentHash.Create(nodes1, 10);
var consistentHash2 = ConsistentHash.Create(nodes2, 10);
var keys = new List<string>(new[] { "A", "B", "C", "D", "E", "F", "G" });

var result1 = keys
.Select(k => consistentHash1.NodeFor(k))
.Select(routee => routee.ToString())
.ToArray();

var result2 = keys
.Select(k => consistentHash2.NodeFor(k))
.Select(routee => routee.ToString())
.ToArray();

result1
.ShouldOnlyContainInOrder(result2);
}
}
}

2 changes: 1 addition & 1 deletion src/core/Akka/Routing/ConsistentHashRouter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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('/') +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it was just the trailing slash that created this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

selection.Selection.PathString;
default:
return Routee.ToString();
Expand Down