curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cammckenzie <...@git.apache.org>
Subject [GitHub] curator pull request #262: [CURATOR-460] Timed tolerance for connection susp...
Date Wed, 04 Apr 2018 01:28:45 GMT
Github user cammckenzie commented on a diff in the pull request:

    https://github.com/apache/curator/pull/262#discussion_r179001451
  
    --- Diff: curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java
---
    @@ -222,6 +223,35 @@ public void notLeader()
                 next.add(states.poll(timing.forSessionSleep().milliseconds(), TimeUnit.MILLISECONDS));
                 next.add(states.poll(timing.forSessionSleep().milliseconds(), TimeUnit.MILLISECONDS));
                 Assert.assertTrue(next.equals(Arrays.asList(ConnectionState.LOST.name(),
"false")) || next.equals(Arrays.asList("false", ConnectionState.LOST.name())), next.toString());
    +
    +            latch.close();
    +            client.close();
    +
    +            timing.sleepABit();
    +            states.clear();
    +            server = new TestingServer();
    +            client = CuratorFrameworkFactory.builder()
    +                    .connectString(server.getConnectString())
    +                    .connectionTimeoutMs(1000)
    +                    .sessionTimeoutMs(timing.session())
    +                    .retryPolicy(new RetryOneTime(1))
    +                    .connectionStateErrorPolicy(new SessionConnectionStateErrorPolicy())
    +                    .connectionHandlingPolicy(new StandardConnectionHandlingPolicy(30))
    +                    .build();
    +            client.getConnectionStateListenable().addListener(stateListener);
    +            client.start();
    +            latch = new LeaderLatch(client, "/test");
    +            latch.addListener(listener);
    +            latch.start();
    +            Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(), TimeUnit.MILLISECONDS),
ConnectionState.CONNECTED.name());
    +            Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(), TimeUnit.MILLISECONDS),
"true");
    +            server.close();
    +            Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(), TimeUnit.MILLISECONDS),
ConnectionState.SUSPENDED.name());
    +            next = Lists.newArrayList();
    +
    +            next.add(states.poll(timing.session() / 3, TimeUnit.MILLISECONDS));
    +            next.add(states.poll(timing.forSleepingABit().milliseconds(), TimeUnit.MILLISECONDS));
    +            Assert.assertTrue(next.equals(Arrays.asList(ConnectionState.LOST.name(),
"false")) || next.equals(Arrays.asList("false", ConnectionState.LOST.name())), next.toString());
    --- End diff --
    
    The issue isn't really the LeaderLatch specifically is it? I think it's probably cleaner
to create a test case under curator-framework for the ConnectionStateManager and then have
a test explicitly for injected LOST events, rather than have something in the LeaderLatch
tests.
    
    Because, this test doesn't really cover the use case that was discussed originally (2
clients being leader at the same time).


---

Mime
View raw message