kezhuw commented on code in PR #500:
URL: https://github.com/apache/curator/pull/500#discussion_r1598125516
##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java:
##########
@@ -208,6 +213,59 @@ public void testWatchedNodeDeletedOnReconnect() throws
Exception {
}
}
+ @Test
+ public void testSessionInterruptionDoNotCauseBrainSplit() throws Exception
{
+ final String latchPath =
"/testSessionInterruptionDoNotCauseBrainSplit";
+ final Timing2 timing = new Timing2();
+ final BlockingQueue<TestEvent> events = new
LinkedBlockingQueue<TestEvent>() {
+ @Override
+ public boolean add(@Nonnull TestEvent testEvent) {
+ LOG.debug("Add event: {}", testEvent);
+ return super.add(testEvent);
+ }
+ };
+
+ final List<Closeable> closeableResources = new ArrayList<>();
+ try {
+ final String id0 = "id0";
+ final CuratorFramework client0 =
createAndStartClient(server.getConnectString(), timing, id0, null);
+ closeableResources.add(client0);
+ final LeaderLatch latch0 = createAndStartLeaderLatch(client0,
latchPath, id0, events);
+ closeableResources.add(latch0);
+
+ assertThat(events.poll(timing.forWaiting().milliseconds(),
TimeUnit.MILLISECONDS))
+ .isNotNull()
+ .isEqualTo(new TestEvent(id0,
TestEventType.GAINED_LEADERSHIP));
+
+ final String id1 = "id1";
+ final CuratorFramework client1 =
createAndStartClient(server.getConnectString(), timing, id1, null);
+ closeableResources.add(client1);
+ final LeaderLatch latch1 = createAndStartLeaderLatch(client1,
latchPath, id1, events);
+ closeableResources.add(latch1);
+
+ // wait for the non-leading LeaderLatch (i.e. latch1) instance to
be done with its creation
+ // this call is time-consuming but necessary because we don't have
a handle to detect the end of the reset
+ // call
+ timing.forWaiting().sleepABit();
+
+ assertTrue(latch0.hasLeadership());
+ assertFalse(latch1.hasLeadership());
+
+
client0.getZookeeperClient().getZooKeeper().getTestable().injectSessionExpiration();
+
+ assertThat(events.poll(timing.forWaiting().milliseconds(),
TimeUnit.MILLISECONDS))
Review Comment:
Is it better to separate events for latch0 and latch1 and assert that the
event (whatever it is) after `LOST_LEADERSHIP` is not equal to `new
TestEvent(id0, TestEventType.GAINED_LEADERSHIP)` ? This way it might be clear
that we are asserting that no `GRAINED_LEADERSHIP` for `id0` after session
changed.
I was thinking this could pass the old code without a run and realized that
if failed in old code since there was a `GRAINED_LEADERSHIP` to `id0` between
the two.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@curator.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
