Akka.net: Clustered Broadcast Group Routers do not seem to work correctly

[mrrd]

I have been wrestling with clustering the broadcast group router.

I have written a testing spec to verify my findings that it just does not seem to work currently, and better visualise that.

The following repository contains my broadcast group clustering test spec:

https://github.com/mrrd/AkkaBroadcastGroupTests

This seems to show that the clustered broadcast-group is only adding a single node routee to it, it should really be two as far as I can see.

[kostrse]

Seems I'm observing the exactly same issue with broadcast-group routers.
Message being sent only to one routee

[ITHedgeHog]

@kostrse What version of the .Net Framework are you running against? mrrd's test runs for me on 4.5.2, but not 4.5 as it was. @Aaronontheweb I've checked the documentation, but I couldn't see where it specified what version of .Net we should be targeting? Does Akka.net only work on .Net 4.5.2?

[kostrse]

@ITHedgeHog 4.6

[kostrse]

The problem is with default value of max-total-nr-of-instances.
When it isn't explicitly set in config, router interpret it as 1. Hence only one routee.

My repro spec: https://github.com/kostrse/akka.net/commit/f1ed00a1d412d616ee390aa6d47df9880a0fbca2 (I simplified it for readability)

[Aaronontheweb]

Akka.NET is compiled against 4.5.

I'll start taking a look at this.

[Aaronontheweb]

The problem is with default value of max-total-nr-of-instances.

Hmmm... sounds like you've correctly described the problem, but this configuration default seems outright wrong. max # of instances for a group router should always be equal to the number of nodes.

[kostrse]

I'd like to think that if I didn't set any explicit limit in config file, then it's not limited (i.e. all members of the cluster).

The default value which set to 10000 is good I think. It just needs to be parsed correctly.
https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka.Cluster/Configuration/Cluster.conf#L199

[mrrd]

@kostrse I have tried adding a value of max-total-nr-of-instances = 9999 to the test spec config but the testing for only 1 routee still passes when this should now really be failing as it should be 2 now shouldn't it?

FYI @Aaronontheweb

[kostrse]

@mrrd I'm not sure, but let me guess.

In your test case, your router is Lazy.
The first time you accessing the router's value is inside of the assertion method, hence that's the place where it being created:
https://github.com/mrrd/AkkaBroadcastGroupTests/blob/master/ClusterBroadcastGroupSpec.cs#L146

I guess again, router needs some tiny moment of time to receive gossip about cluster members (also asynchronously), so at the moment when you asking a list of routees - it doesn't know about other members yet.

Just try to set Should().Be(2), probably it will work too.

[mrrd]

@kostrse that seems to be it, if I initialise the router before trying to get the count using a barrier it now passes with checking for two and also setting "max-total-nr-of-instances", I can also confirm where the problem lies that the default it is retrieving is also 1 for that value if it is not specifically set because I also then removed that setting and it goes back to failing on checking for 2 but succeeding on 1.

[mrrd]

@kostrse my next job for tomorrow will be to try this config work around in my app to verify that it now functions as expected, thanks for your help getting to the bottom of this bug, just glad there is now a workaround I can try.

[Aaronontheweb]

I'd like to think that if I didn't set any explicit limit in config file, then it's not limited (i.e. all members of the cluster).

I agree. The upper limit should be unbounded by default.

[eisendle]

I also tried to create a clustered broadcast group router and had the same problems.
Setting nr-of-instances to a value greater than 1 didn`t help. However setting max-nr-of-instances-per-node to 100 did the trick. Messages got routed to all routees.
This contradicts the documentation (http://getakka.net/docs/clustering/cluster-routing#cluster-router-config), so I suppose this is a bug.

[Aaronontheweb]

I'm on the case

[Aaronontheweb]

Found one issue that would for-sure cause this, although it might not be the _only_ reason.

internal abstract class ClusterRouterActor : RouterActor
    {
        protected ClusterRouterActor(ClusterRouterSettingsBase settings)
        {
            Settings = settings;

            if (!(Cell.RouterConfig is Pool) && !(Cell.RouterConfig is Group))
            {
                throw new ActorInitializationException(string.Format("Cluster router actor can only be used with Pool or Group, not with {0}", Cell.RouterConfig.GetType()));
            }

            Cluster = Cluster.Get(Context.System);
            Nodes = ImmutableSortedSet.Create(Member.AddressOrdering, Cluster.ReadView.Members.Where(IsAvailable).Select(x => x.Address).ToArray());
        }

        public ClusterRouterSettingsBase Settings { get; protected set; }

        public Cluster Cluster { get; }

        protected override void PreStart()
        {
            Cluster.Subscribe(Self, new[]
            {
                typeof(ClusterEvent.IMemberEvent),
                typeof(ClusterEvent.IReachabilityEvent)
            });
        }

        protected override void PostStop()
        {
            Cluster.Unsubscribe(Self);
        }

        public ImmutableSortedSet<Address> Nodes { get; private set; }

[Aaronontheweb]

In the comment above, Nodes is a mutable property that has zero concurrency mechanisms to protect it when the ClusterRouterActor is processing events from the cluster. No CAS or anything. It's dangerous mutable state.

Looks like on the JVM this has been updated to use the ClusterReadView, which is smart and also avoids "slow start" problems that clustered routers often have (users send a message to the router immediately after it has been created, but before it's received CurrentClusterState from the Cluster - so no routees.)

I'm going to port @mrrd's reproduction spec into the Akka.Cluster.Tests.MultiNode library and debug it from there.

[Aaronontheweb]

I take that back. This operation inside the actor is safe as it passes through a mailbox that is preserved for the actor that controls the router. So it's something else. Looking at the configuration area now.

[Aaronontheweb]

It's the configuration as @kostrse suggested.

Reproduction spec:

[Fact]
        public void BugFix2266RemoteDeployer_must_be_able_to_parse_broadcast_group_cluster_router_with_default_nr_of_routees_routees()
        {
            var service = "/user/service3";
            var deployment = Sys.AsInstanceOf<ActorSystemImpl>().Provider.Deployer.Lookup(service.Split('/').Drop(1));
            deployment.Should().NotBeNull();

            deployment.Path.ShouldBe(service);
            deployment.RouterConfig.GetType().ShouldBe(typeof(ClusterRouterGroup));
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.GetType().ShouldBe(typeof(BroadcastGroup));
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.AsInstanceOf<BroadcastGroup>().Paths.ShouldBe(new[] { "/user/myservice" });
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.TotalInstances.ShouldBe(10000);
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.AllowLocalRoutees.ShouldBe(false);
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.UseRole.ShouldBe("backend");
            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.AsInstanceOf<ClusterRouterGroupSettings>().RouteesPaths.ShouldBe(new[] { "/user/myservice" });
            deployment.Scope.ShouldBe(ClusterScope.Instance);
            deployment.Mailbox.ShouldBe("mymailbox");
            deployment.Dispatcher.ShouldBe("mydispatcher");
        }

Only 1 routee.