Fixed cluster group router UseRole ignored bug #3303

Aaronontheweb · 2018-01-29T22:42:04Z

Addresses #3294

Aaronontheweb · 2018-01-29T22:43:22Z

src/core/Akka.Cluster.Tests.MultiNode/Routing/UseRoleIgnoredSpec.cs

@@ -272,7 +272,7 @@ private void A_cluster_must_group_local_on_role_b()
                var router = Sys.ActorOf(
                    new ClusterRouterGroup(
                        new RoundRobinGroup(paths: null),
-                        new ClusterRouterGroupSettings(6, ImmutableHashSet.Create("/user/foo", "/user/bar"), allowLocalRoutees: false, useRole: role)).Props(),
+                        new ClusterRouterGroupSettings(6, ImmutableHashSet.Create("/user/foo", "/user/bar"), allowLocalRoutees: true, useRole: role)).Props(),


As it turns out, this is a bug. allowLocalRoutees is supposed to be set for true for this part of the spec, per the JVM implementation for router-3b:

https://github.com/akka/akka/blob/a94f7cdc98332875e21900017c7eac52dbfa2bbd/akka-cluster/src/multi-jvm/scala/akka/cluster/routing/UseRoleIgnoredSpec.scala#L193

ClusterRouterGroupSettings(totalInstances = 6, routeesPaths = List("/user/foo", "/user/bar"), allowLocalRoutees = true, useRole = role)).props, "router-3b")

This is why this spec didn't catch the bug earlier. The two bugs cancelled eachother out.

Aaronontheweb · 2018-01-29T22:43:57Z

src/core/Akka/Routing/RouterConfig.cs

        /// </summary>
+        [Obsolete("Deprecated since Akka.NET v1.1. Use Paths(ActorSystem) instead.")]


Added note to indicate that this method is now obsolete, which it is.

Aaronontheweb · 2018-01-29T22:49:06Z

Error logs upon re-running this spec with my changes:

[Node1:first][FAIL] Akka.Cluster.Tests.MultiNode.Routing.UseRoleIgnoredSpec.UseRoleIgnoredSpecs
[Node1:first][FAIL-EXCEPTION] Type: Xunit.Sdk.XunitException
--> [Node1:first][FAIL-EXCEPTION] Message: Expected value to be 4, but found 6.
--> [Node1:first][FAIL-EXCEPTION] StackTrace:    at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
   at FluentAssertions.Numeric.NumericAssertions`1.Be(T expected, String because, Object[] becauseArgs)
   at Akka.TestKit.TestKitBase.AwaitAssert(Action assertion, Nullable`1 duration, Nullable`1 interval) in C:\Repositories\olympus\akka.net\src\core\Akka.TestKit\TestKitBase_AwaitAssert.cs:line 51
   at Akka.Cluster.Tests.MultiNode.Routing.UseRoleIgnoredSpec.<A_cluster_must_group_local_on_role_b>b__11_0() in C:\Repositories\olympus\akka.net\src\core\Akka.Cluster.Tests.MultiNode\Routing\UseRoleIgnoredSpec.cs:line 281
   at Akka.Remote.TestKit.MultiNodeSpec.RunOn(Action thunk, RoleName[] nodes) in C:\Repositories\olympus\akka.net\src\core\Akka.Remote.TestKit\MultiNodeSpec.cs:line 508
   at Akka.Cluster.Tests.MultiNode.Routing.UseRoleIgnoredSpec.A_cluster_must_group_local_on_role_b() in C:\Repositories\olympus\akka.net\src\core\Akka.Cluster.Tests.MultiNode\Routing\UseRoleIgnoredSpec.cs:line 295
   at Akka.Cluster.Tests.MultiNode.Routing.UseRoleIgnoredSpec.UseRoleIgnoredSpecs() in C:\Repositories\olympus\akka.net\src\core\Akka.Cluster.Tests.MultiNode\Routing\UseRoleIgnoredSpec.cs:line 136

[ERROR][1/29/2018 10:34:16 PM][Thread 0049][akka://NoteTestRunner-0/system/IO-TCP/$a] Monitored actor [[akka://NoteTestRunner-0/user/$a#1543076061]] terminated
Cause: Akka.Actor.DeathPactException: Monitored actor [[akka://NoteTestRunner-0/user/$a#1543076061]] terminated
   at Akka.Actor.ActorBase.Unhandled(Object message) in C:\Repositories\olympus\akka.net\src\core\Akka\Actor\ActorBase.cs:line 193
   at Akka.Actor.ActorBase.AroundReceive(Receive receive, Object message) in C:\Repositories\olympus\akka.net\src\core\Akka\Actor\ActorBase.cs:line 158
   at Akka.Actor.ActorCell.ReceiveMessage(Object message) in C:\Repositories\olympus\akka.net\src\core\Akka\Actor\ActorCell.DefaultMessages.cs:line 179
   at Akka.Actor.ActorCell.AutoReceiveMessage(Envelope envelope) in C:\Repositories\olympus\akka.net\src\core\Akka\Actor\ActorCell.DefaultMessages.cs:line 145
   at Akka.Actor.ActorCell.Invoke(Envelope envelope) in C:\Repositories\olympus\akka.net\src\core\Akka\Actor\ActorCell.DefaultMessages.cs:line 77

…ter implementations

Aaronontheweb · 2018-01-30T12:53:42Z

src/core/Akka/Routing/RouterConfig.cs

@@ -145,13 +145,20 @@ public abstract class Group : RouterConfig, IEquatable<Group>
        /// <param name="routerDispatcher">TBD</param>
        protected Group(IEnumerable<string> paths, string routerDispatcher) : base(routerDispatcher)
        {
-            Paths = paths;


This is the real fix here. Needed to stop using the deprecated Paths property so the router will fall back to calling the GetPaths(ActorSystem) method instead, which does the right thing. Created a protected _paths property to hold onto the value that will be used inside that method in most built-in routers, which is identical to what the JVM has.

Horusiath · 2018-01-30T21:54:52Z

src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt

@@ -4061,7 +4061,9 @@ namespace Akka.Routing
    }
    public abstract class Group : Akka.Routing.RouterConfig, System.IEquatable<Akka.Routing.Group>
    {
+        protected readonly string[] _paths;


Is it really intended? ;)

Yep - the value has to be stored somewhere in order to be propagated inside local routers. We kind of effed ourselves by using an automatic property to set the value before.

If it's intended to work this way, please correct the naming conventions.

@Horusiath done - renamed it to fix the ReSharper conventions.

Horusiath · 2018-01-30T21:56:31Z

src/core/Akka/Routing/RoutedActorCell.cs

@@ -167,7 +167,7 @@ public override void Start()
                var deprecatedPaths = group.Paths;

                var paths = deprecatedPaths == null
-                        ? group.GetPaths(System).ToArray()
+                        ? group.GetPaths(System)?.ToArray()


The next line is call to paths.NonEmpty() - if we're accepting to have paths to be potentially null, this could end with NRE.

paths.NonEmpty() has a null-check built into it.

…Aaronontheweb/akka.net into fix-3294-use-role-ignored-group

Aaronontheweb · 2018-01-31T02:42:36Z

Bah, forgot to do API approvals

* fixed bug in UseRoleIgnoredSpec * close #3294 - fixed usages of GetPaths inside all Group router implementations

fixed bug in UseRoleIgnoredSpec

9df1af9

Aaronontheweb added the needs review label Jan 29, 2018

Aaronontheweb commented Jan 29, 2018

View reviewed changes

Aaronontheweb added 3 commits January 29, 2018 17:16

close akkadotnet#3294 - fixed usages of GetPaths inside all Group rou…

d74da7c

…ter implementations

approved API and fixed NullReferenceExceptions

1b3e18c

fixed final NRE issue

de10b18

Aaronontheweb changed the title ~~[WIP] fixing cluster group router UseRole ignored bug~~ Fixed cluster group router UseRole ignored bug Jan 30, 2018

Aaronontheweb added the ready label Jan 30, 2018

Aaronontheweb commented Jan 30, 2018

View reviewed changes

Merge branch 'dev' into fix-3294-use-role-ignored-group

2eb9949

Horusiath requested changes Jan 30, 2018

View reviewed changes

Aaronontheweb added 4 commits January 30, 2018 16:20

style cleanup

b6b720b

Merge branch 'fix-3294-use-role-ignored-group' of https://github.com/…

ce37417

…Aaronontheweb/akka.net into fix-3294-use-role-ignored-group

commited change to RouterConfig.cs

6b023f5

committed R#er changes to RoundRobin.cs

8d669f2

Horusiath approved these changes Jan 31, 2018

View reviewed changes

updated API approval

e3dab64

Aaronontheweb merged commit f45adfa into akkadotnet:dev Jan 31, 2018

Aaronontheweb deleted the fix-3294-use-role-ignored-group branch January 31, 2018 17:27

Aaronontheweb removed needs review ready labels Jan 31, 2018

Aaronontheweb added a commit that referenced this pull request Feb 1, 2018

Fixed cluster group router UseRole ignored bug (#3303)

f028ab1

* fixed bug in UseRoleIgnoredSpec * close #3294 - fixed usages of GetPaths inside all Group router implementations

Aaronontheweb added a commit that referenced this pull request Feb 19, 2018

Fixed cluster group router UseRole ignored bug (#3303)

96cd99a

* fixed bug in UseRoleIgnoredSpec * close #3294 - fixed usages of GetPaths inside all Group router implementations

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed cluster group router UseRole ignored bug #3303

Fixed cluster group router UseRole ignored bug #3303

Aaronontheweb commented Jan 29, 2018

Aaronontheweb Jan 29, 2018

Aaronontheweb Jan 29, 2018

Aaronontheweb commented Jan 29, 2018

Aaronontheweb Jan 30, 2018

Horusiath Jan 30, 2018

Aaronontheweb Jan 30, 2018

Horusiath Jan 30, 2018

Aaronontheweb Jan 30, 2018

Horusiath Jan 30, 2018

Aaronontheweb Jan 30, 2018

Aaronontheweb commented Jan 31, 2018

		/// </summary>
		[Obsolete("Deprecated since Akka.NET v1.1. Use Paths(ActorSystem) instead.")]

Fixed cluster group router UseRole ignored bug #3303

Fixed cluster group router UseRole ignored bug #3303

Conversation

Aaronontheweb commented Jan 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Jan 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Jan 31, 2018