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.

Sign up for GitHub

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

Jump to bottom

Member status: Weakly up #3099

Merged
merged 7 commits into from
Dec 21, 2017
Merged

Member status: Weakly up #3099

merged 7 commits into from
Dec 21, 2017

Conversation

Horusiath
Copy link
Contributor

No description provided.

@Horusiath Horusiath added this to the 1.4.0 milestone Sep 15, 2017
@Horusiath
Copy link
Contributor Author

One note: if we're going to release v1.3.2, don't merge it yet. It's too serious to land in patch release, it's meant to for v1.4.

@Aaronontheweb
Copy link
Member

1.4 aint happen for a while. Need a feature branch then.

@Horusiath Horusiath changed the base branch from dev to v1.4 September 19, 2017 06:47
@Horusiath Horusiath force-pushed the weakly-up branch 2 times, most recently from 976608a to d4c792f Compare October 15, 2017 07:28
@Horusiath
Copy link
Contributor Author

Got some Hyperion serialization exceptions of MNTK for windows .NET core - something related to cluster sharding.

@Aaronontheweb
Copy link
Member

Aaronontheweb commented Oct 15, 2017 via email

@Horusiath Horusiath mentioned this pull request Oct 18, 2017
Closed
@Horusiath Horusiath changed the base branch from v1.4 to dev December 4, 2017 18:10
@Horusiath
Copy link
Contributor Author

Horusiath commented Dec 4, 2017 •
edited
Loading

I've turned this feature off by default - this way it won't affect current cluster protocol, and it still can be set manually.

@Horusiath Horusiath removed this from the 1.4.0 milestone Dec 6, 2017
@Aaronontheweb
Copy link
Member

Aaronontheweb commented Dec 7, 2017 •
edited
Loading

I need to check:

Aaronontheweb
Aaronontheweb requested changes Dec 7, 2017
View reviewed changes
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

None of the clustering MNTR specs actually ran. Just the Cluster.Tools one. Can't merge it until we see those pass. No idea why - must be the incremental build stuff.

@Aaronontheweb
Copy link
Member

Looks like we're in the clear on the node leader transition stuff, since you added WeaklyUp to the AllowedTransitions table, which is what our leader picking algorithm uses to determine what's the correct "next state" for each node in transition.

@Aaronontheweb
Copy link
Member

Aaronontheweb commented Dec 21, 2017 •
edited
Loading

Doesn't affect leader elections either:

   private static readonly ImmutableHashSet<MemberStatus> LeaderMemberStatus =
            ImmutableHashSet.Create(MemberStatus.Up, MemberStatus.Leaving);

WeaklyUp members can't be leaders per these changes.

@Aaronontheweb
Copy link
Member

Looks good to me. We'll get this in for v1.3.3

@Aaronontheweb Aaronontheweb merged commit a5c5b65 into akkadotnet:dev Dec 21, 2017
@Aaronontheweb Aaronontheweb mentioned this pull request Jan 16, 2018
Closed
Aaronontheweb
Aaronontheweb reviewed Jan 16, 2018
View reviewed changes
src/core/Akka.Cluster/ClusterDaemon.cs
if (!changedMembers.IsEmpty)
{
// replace changed members
var newMembers = changedMembers.Union(localMembers);
Copy link
Member

Choose a reason for hiding this comment

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

Son of a bitch... I totally missed this when I originally reviewed the PR. It's going to great the same problems here that happened in #2584. This is a bug. I'll PR it per #3274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aaronontheweb Aaronontheweb Aaronontheweb approved these changes

Assignees
No one assigned
Labels
akka-cluster
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Horusiath @Aaronontheweb