Cluster singleton should consider Member AppVersion during hand over. #6065
Conversation
| } | ||
|
|
||
| public override bool Equals(object obj) | ||
| { | ||
| return base.Equals(obj as AppVersion); |
There was a problem hiding this comment.
Since AppVersion extends Object directly, base.Equals() does a direct reference equality, which is wrong.
| // 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; |
There was a problem hiding this comment.
AppVersion difference always wins compared to Member age difference
| _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\"") |
| // 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"); |
| @@ -880,7 +880,9 @@ private void InitializeFSM() | |||
| { | |||
| case StartOldestChangedBuffer _: | |||
| { | |||
| _oldestChangedBuffer = Context.ActorOf(Actor.Props.Create<OldestChangedBuffer>(_settings.Role).WithDispatcher(Context.Props.Dispatcher)); | |||
| _oldestChangedBuffer = Context.ActorOf( | |||
| Actor.Props.Create(() => new OldestChangedBuffer(_settings.Role, _settings.ConsiderAppVersion)) | |||
| { | ||
| // prefer nodes with the highest app version, even if they're younger | ||
| var appVersionDiff = x.AppVersion.CompareTo(y.AppVersion); | ||
| if (appVersionDiff != 0) |
There was a problem hiding this comment.
So we have to run the sort in two phases here:
- AppVersion
- Age within the AppVersion cohort.
Looks to me like the sorter disregards age when AppVersion is considered - is that correct?
There was a problem hiding this comment.
Yes, that is correct, AppVersion always wins, we only consider member age if their AppVersion matches
There was a problem hiding this comment.
This design is wrong then - without the member age requirement there's no guarantee of stable singleton placement, which defeats the purpose of adding this feature (i.e. being able to predict the next location of where a singleton will be placed during a deployment)
We need to do the sort in multiple phases:
- Members by role
- Members within a role that have the highest app version
- Oldest member within the results of phase 2.
Let's design it to work that way.
src/contrib/cluster/Akka.Cluster.Tools.Tests/Singleton/MemberAgeOrderingSpec.cs
Show resolved
Hide resolved
| { | ||
| Create(Address.Parse("akka://sys@darkstar:1112"), upNumber: 3, appVersion: AppVersion.Create("1.0.0")), | ||
| Create(Address.Parse("akka://sys@darkstar:1113"), upNumber: 1, appVersion: AppVersion.Create("1.0.0")), | ||
| Create(Address.Parse("akka://sys@darkstar:1116"), upNumber: 6, appVersion: AppVersion.Create("1.0.2")), |
There was a problem hiding this comment.
Could you humor me and add this item last on this list? That'd prove that the upnumber is canon, not insertion order.
|
TODO: need to add changelog and website documentation for this feature edit: in a new PR |
|
@Arkatufus API approvals are out of date |
Fixes #6035
Changes
Changes the way oldest member sorting comparator works, taking Member.AppVersion into consideration. Members with higher AppVersion will always be bumped to the front of the list, regardless of Member age.