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

Cluster singleton should consider Member AppVersion during hand over. #6065

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
//-----------------------------------------------------------------------
// <copyright file="ClusterSingletonRestart2Spec.cs" company="Akka.NET Project">
// Copyright (C) 2009-2021 Lightbend Inc. <http://www.lightbend.com>
// Copyright (C) 2013-2021 .NET Foundation <https://github.com/akkadotnet/akka.net>
// </copyright>
//-----------------------------------------------------------------------

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;
using Akka.Actor;
using Akka.Cluster.Tools.Singleton;
using Akka.Configuration;
using Akka.TestKit;
using FluentAssertions;
using Xunit;
using Xunit.Abstractions;

namespace Akka.Cluster.Tools.Tests.Singleton
{
public class ClusterSingletonRestart3Spec : AkkaSpec
{
private readonly ActorSystem _sys1;
private readonly ActorSystem _sys2;
private readonly ActorSystem _sys3;

public ClusterSingletonRestart3Spec(ITestOutputHelper output) : base(@"
akka.loglevel = DEBUG
akka.actor.provider = ""cluster""
akka.cluster.app-version = ""1.0.0""
akka.cluster.auto-down-unreachable-after = 2s
akka.cluster.singleton.min-number-of-hand-over-retries = 5
akka.remote {
dot-netty.tcp {
hostname = ""127.0.0.1""
port = 0
}
}", output)
{
_sys1 = Sys;
_sys2 = ActorSystem.Create(Sys.Name, Sys.Settings.Config);
InitializeLogger(_sys2);
_sys3 = ActorSystem.Create(Sys.Name, ConfigurationFactory.ParseString("akka.cluster.app-version = \"1.0.2\"")
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

.WithFallback(Sys.Settings.Config));
InitializeLogger(_sys3);
}

public void Join(ActorSystem from, ActorSystem to)
{
from.ActorOf(ClusterSingletonManager.Props(Props.Create(() => new Singleton()),
PoisonPill.Instance,
ClusterSingletonManagerSettings.Create(from)), "echo");


Within(TimeSpan.FromSeconds(45), () =>
{
AwaitAssert(() =>
{
Cluster.Get(from).Join(Cluster.Get(to).SelfAddress);
Cluster.Get(from).State.Members.Select(x => x.UniqueAddress).Should().Contain(Cluster.Get(from).SelfUniqueAddress);
Cluster.Get(from)
.State.Members.Select(x => x.Status)
.ToImmutableHashSet()
.Should()
.Equal(ImmutableHashSet<MemberStatus>.Empty.Add(MemberStatus.Up));
});
});
}

[Fact]
public void Singleton_should_consider_AppVersion_when_handing_over()
{
Join(_sys1, _sys1);
Join(_sys2, _sys1);

var proxy2 = _sys2.ActorOf(
ClusterSingletonProxy.Props("user/echo", ClusterSingletonProxySettings.Create(_sys2)), "proxy2");

Within(TimeSpan.FromSeconds(5), () =>
{
AwaitAssert(() =>
{
var probe = CreateTestProbe(_sys2);
proxy2.Tell("poke", probe.Ref);
var singleton = probe.ExpectMsg<Member>(TimeSpan.FromSeconds(1));
singleton.Should().Be(Cluster.Get(_sys1).SelfMember);
singleton.AppVersion.Version.Should().Be("1.0.0");
});
});

// A new node with higher AppVersion joins the cluster
Join(_sys3, _sys1);

// Old node with the singleton instance left the cluster
Cluster.Get(_sys1).Leave(Cluster.Get(_sys1).SelfAddress);

// let it stabilize
Task.Delay(TimeSpan.FromSeconds(5)).Wait();

Within(TimeSpan.FromSeconds(10), () =>
{
AwaitAssert(() =>
{
var probe = CreateTestProbe(_sys2);
proxy2.Tell("poke", probe.Ref);

// note that _sys3 has a higher app-version, so the singleton should start there
var singleton = probe.ExpectMsg<Member>(TimeSpan.FromSeconds(1));
singleton.Should().Be(Cluster.Get(_sys3).SelfMember);
singleton.AppVersion.Version.Should().Be("1.0.2");
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

});
});
}

protected override async Task AfterAllAsync()
{
await base.AfterAllAsync();
await ShutdownAsync(_sys2);
await ShutdownAsync(_sys3);
}

public class Singleton : ReceiveActor
{
public Singleton()
{
ReceiveAny(o =>
{
Sender.Tell(Cluster.Get(Context.System).SelfMember);
});
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ private MemberAgeOrdering(bool ascending)
/// <inheritdoc/>
public int Compare(Member x, Member y)
{
if (x is null && y is null)
return 0;

if (y is null)
return _ascending ? 1 : -1;

if (x is null)
return _ascending ? -1 : 1;

// prefer nodes with the highest app version, even if they're younger
var appVersionDiff = x.AppVersion.CompareTo(y.AppVersion);
if (appVersionDiff != 0)
return _ascending ? appVersionDiff : appVersionDiff * -1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AppVersion difference always wins compared to Member age difference


if (x.Equals(y)) return 0;
return x.IsOlderThan(y)
? (_ascending ? 1 : -1)
Expand Down
8 changes: 4 additions & 4 deletions src/core/Akka/Util/AppVersion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ private AppVersion Parse()

public int CompareTo(AppVersion other)
{
if (Version == other.Version) // String equals without requiring parse
if (string.Equals(Version, other.Version, StringComparison.Ordinal)) // String equals without requiring parse
return 0;
else
{
Expand All @@ -232,7 +232,7 @@ public int CompareTo(AppVersion other)
if (other._rest == "" && _rest != "")
diff = -1;
else
diff = _rest.CompareTo(other._rest);
diff = string.Compare(_rest, other._rest, StringComparison.Ordinal);
}
}
}
Expand All @@ -243,12 +243,12 @@ public int CompareTo(AppVersion other)

public bool Equals(AppVersion other)
{
return other != null && Version == other.Version;
return other != null && string.Equals(Version, other.Version, StringComparison.Ordinal);
}

public override bool Equals(object obj)
{
return base.Equals(obj as AppVersion);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since AppVersion extends Object directly, base.Equals() does a direct reference equality, which is wrong.

return obj is AppVersion av && Equals(av);
}

public static bool operator ==(AppVersion first, AppVersion second)
Expand Down