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

Implemented LeaderDowningAllOtherNodesSpec #2099

Merged
merged 1 commit into from
Jun 30, 2016
Merged

Implemented LeaderDowningAllOtherNodesSpec #2099

merged 1 commit into from
Jun 30, 2016

Conversation

alexvaluyskiy
Copy link
Contributor

@alexvaluyskiy alexvaluyskiy commented Jun 19, 2016 •
edited
Loading

Reviewed these cluster specs:

  • ClusterHeartbeatSenderStateSpec
  • HeartbeatNodeRingSpec

@alexvaluyskiy alexvaluyskiy mentioned this pull request Jun 20, 2016
Merged
@Aaronontheweb
Copy link
Member

Weird... why did CI not pick this up?

@Aaronontheweb
Copy link
Member

Ah, I see why - it shows the commit as being 20 days old even though you only submitted the PR yesterday. Therefore TeamCity won't try to pick it up.

You can reset the git timestamp using this technique: http://stackoverflow.com/a/31540373/377476

@alexvaluyskiy alexvaluyskiy force-pushed the LeaderDowningAllOtherNodesSpec branch 2 times, most recently from 1bae754 to 68540b3 Compare June 21, 2016 18:52
@Aaronontheweb
Copy link
Member

Failure on Node 1 for this spec

[Node1][FAIL] Akka.Cluster.Tests.MultiNode.LeaderDowningAllOtherNodesSpecNode1.LeaderDowningAllOtherNodesSpecs
[Node1][FAIL-EXCEPTION] Type: Xunit.Sdk.FalseException
--> [Node1][FAIL-EXCEPTION] Message: Assert.False() Failure
Expected: False
Actual:   True
--> [Node1][FAIL-EXCEPTION] StackTrace:    at Xunit.Assert.False(Nullable`1 condition, String userMessage) in c:\TeamCity\buildAgent\work\74856245f07a90f0\src\xunit.assert\Asserts\BooleanAsserts.cs:line 47
   at Akka.Cluster.TestKit.MultiNodeClusterSpec.<>c__DisplayClass26_0.<AwaitMembersUp>b__1() in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Cluster.TestKit\MultiNodeClusterSpec.cs:line 364
   at Akka.TestKit.TestKitBase.AwaitAssert(Action assertion, Nullable`1 duration, Nullable`1 interval) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.TestKit\TestKitBase_AwaitAssert.cs:line 48
   at Akka.Cluster.TestKit.MultiNodeClusterSpec.<>c__DisplayClass26_0.<AwaitMembersUp>b__0() in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Cluster.TestKit\MultiNodeClusterSpec.cs:line 362
   at Akka.TestKit.TestKitBase.<>c__DisplayClass116_0.<Within>b__0() in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.TestKit\TestKitBase_Within.cs:line 38
   at Akka.TestKit.TestKitBase.Within[T](TimeSpan min, TimeSpan max, Func`1 function, String hint) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.TestKit\TestKitBase_Within.cs:line 79
   at Akka.TestKit.TestKitBase.Within(TimeSpan min, TimeSpan max, Action action, String hint) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.TestKit\TestKitBase_Within.cs:line 38
   at Akka.TestKit.TestKitBase.Within(TimeSpan max, Action action) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.TestKit\TestKitBase_Within.cs:line 26
   at Akka.Cluster.TestKit.MultiNodeClusterSpec.AwaitMembersUp(Int32 numbersOfMembers, ImmutableHashSet`1 canNotBePartOfMemberRing, Nullable`1 timeout) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Cluster.TestKit\MultiNodeClusterSpec.cs:line 359
   at Akka.Cluster.Tests.MultiNode.LeaderDowningAllOtherNodesSpec.A_Cluster_of_6_nodes_with_monitored_by_nr_of_members_2_must_remove_all_shutdown_nodes() in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Cluster.Tests.MultiNode\LeaderDowningAllOtherNodesSpec.cs:line 85
   at Akka.Cluster.Tests.MultiNode.LeaderDowningAllOtherNodesSpec.LeaderDowningAllOtherNodesSpecs() in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Cluster.Tests.MultiNode\LeaderDowningAllOtherNodesSpec.cs:line 62

@alexvaluyskiy alexvaluyskiy force-pushed the LeaderDowningAllOtherNodesSpec branch 7 times, most recently from 1ee1f03 to 07909e6 Compare June 29, 2016 16:59
alexvaluyskiy
alexvaluyskiy reviewed Jun 29, 2016
View reviewed changes
src/core/Akka.Cluster/ClusterHeartbeat.cs

public HeartbeatNodeRing(
UniqueAddress selfAddress,
ImmutableHashSet<UniqueAddress> nodes,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed ImmuableSortedSet to ImmutableHashSet

@alexvaluyskiy alexvaluyskiy force-pushed the LeaderDowningAllOtherNodesSpec branch from 42583a1 to 1aeeafa Compare June 29, 2016 18:38
@alexvaluyskiy
Copy link
Contributor Author

@akkadotnet/developers Ready to review

@alexvaluyskiy alexvaluyskiy changed the title [WIP] Implemented LeaderDowningAllOtherNodesSpec Implemented LeaderDowningAllOtherNodesSpec Jun 29, 2016
@Aaronontheweb Aaronontheweb self-assigned this Jun 29, 2016
@Aaronontheweb
Copy link
Member

Looking good - I'll review this tonight.

@alexvaluyskiy
Copy link
Contributor Author

We still have some racy conditions somewhere. But not here

Aaronontheweb
Aaronontheweb reviewed Jun 30, 2016
View reviewed changes
src/core/Akka.Cluster/ClusterHeartbeat.cs

public ImmutableHashSet<UniqueAddress> Unreachable { get; private set; }
public ImmutableHashSet<UniqueAddress> OldReceiversNowUnreachable { get; }
Copy link
Member

Choose a reason for hiding this comment

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

This will require a rebase - the API approval diff for Akka.Cluster will fire on this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because this class is internal

@Aaronontheweb
Copy link
Member

@alexvaluyskiy Looks good to me - needs to be rebased on dev due to #2124; some of the changes in this PR will trigger a diff in the API approval file for Akka.Cluster. Submit a PR with that rebase and I'll merge it.

@alexvaluyskiy
Copy link
Contributor Author

alexvaluyskiy commented Jun 30, 2016 •
edited
Loading

@Aaronontheweb I've changed only internal classes. No public API was changed

@Aaronontheweb
Copy link
Member

Ok then

@Aaronontheweb Aaronontheweb merged commit 6f6378f into akkadotnet:dev Jun 30, 2016
@alexvaluyskiy alexvaluyskiy deleted the LeaderDowningAllOtherNodesSpec branch June 30, 2016 19:49
This was referenced Jun 30, 2016
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Aaronontheweb Aaronontheweb

Labels
rebase required reviewed positively
Projects
None yet
Milestone
No milestone
Development

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