You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ca...@apache.org on 2014/08/20 07:54:56 UTC
[1/4] git commit: CURATOR-141 Don't make calls to the client after
PathChildrenCache has been closed.
Repository: curator
Updated Branches:
refs/heads/CURATOR-141 [created] 0d1397af1
CURATOR-141 Don't make calls to the client after PathChildrenCache has been closed.
The test case is a bit weird because it needed to intercept logging.
Also, I've added slf4j-log4j12 as a test dependency to control logging.
I think it should be added to the other poms as well, otherwise logging
seems to get lost.
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f2f1953c
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f2f1953c
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f2f1953c
Branch: refs/heads/CURATOR-141
Commit: f2f1953c544398cb6db53a2ac59b02719cc83fa3
Parents: d2c37d0
Author: Karel Vervaeke <ka...@ngdata.com>
Authored: Thu Aug 14 13:46:03 2014 +0200
Committer: Karel Vervaeke <ka...@ngdata.com>
Committed: Thu Aug 14 13:46:03 2014 +0200
----------------------------------------------------------------------
curator-recipes/pom.xml | 6 ++
.../recipes/cache/PathChildrenCache.java | 4 +
.../recipes/cache/TestPathChildrenCache.java | 85 +++++++++++++++++++-
pom.xml | 6 ++
4 files changed, 100 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/pom.xml
----------------------------------------------------------------------
diff --git a/curator-recipes/pom.xml b/curator-recipes/pom.xml
index 13aa475..37155c9 100644
--- a/curator-recipes/pom.xml
+++ b/curator-recipes/pom.xml
@@ -61,5 +61,11 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
+
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
index d555508..57c6e92 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
@@ -487,6 +487,10 @@ public class PathChildrenCache implements Closeable
@Override
public void processResult(CuratorFramework client, CuratorEvent event) throws Exception
{
+ if (PathChildrenCache.this.state.get().equals(State.CLOSED)) {
+ // This ship is closed, don't handle the callback
+ return;
+ }
if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
{
processChildren(event.getChildren(), mode);
http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
index 60f2e88..70156ff 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
@@ -18,22 +18,44 @@
*/
package org.apache.curator.framework.recipes.cache;
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Collections2;
import com.google.common.collect.Lists;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.api.UnhandledErrorListener;
+import org.apache.curator.framework.imps.CuratorFrameworkImpl;
import org.apache.curator.retry.RetryOneTime;
import org.apache.curator.test.BaseClassForTests;
import org.apache.curator.test.KillSession;
import org.apache.curator.test.Timing;
import org.apache.curator.utils.CloseableUtils;
+import org.apache.log4j.Appender;
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.apache.log4j.SimpleLayout;
+import org.apache.log4j.spi.LoggingEvent;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.testng.Assert;
import org.testng.annotations.Test;
+
import java.util.Collection;
import java.util.List;
-import java.util.concurrent.*;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Exchanger;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -76,6 +98,67 @@ public class TestPathChildrenCache extends BaseClassForTests
}
@Test
+ public void testClientClosedDuringRefreshErrorMessage() throws Exception
+ {
+ Timing timing = new Timing();
+
+ // Fiddle with logging so we can intercept the error events for org.apache.curator
+ final List<LoggingEvent> events = Lists.newArrayList();
+ Collection<String> messages = Collections2.transform(events, new Function<LoggingEvent, String>() {
+ @Override
+ public String apply(LoggingEvent loggingEvent) {
+ return loggingEvent.getRenderedMessage();
+ }
+ });
+ Appender appender = new AppenderSkeleton(true) {
+ @Override
+ protected void append(LoggingEvent event) {
+ if (event.getLevel().equals(Level.ERROR)) {
+ events.add(event);
+ }
+ }
+
+ @Override
+ public void close() {
+
+ }
+
+ @Override
+ public boolean requiresLayout() {
+ return false;
+ }
+ };
+ appender.setLayout(new SimpleLayout());
+ Logger rootLogger = Logger.getLogger("org.apache.curator");
+ rootLogger.addAppender(appender);
+
+ // Check that the logging setup didn't change in a way that breaks this test
+ Logger.getLogger(CuratorFrameworkImpl.class).error("Just checking");
+ Assert.assertTrue(messages.contains("Just checking"),
+ "The test error message was not logged, this test may be broken");
+
+ // try to reproduce a bunch of times because it doesn't happen reliably
+ for (int i = 0; i < 50; i++) {
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ client.start();
+ try {
+ PathChildrenCache cache = new PathChildrenCache(client, "/test", true);
+ cache.start(PathChildrenCache.StartMode.BUILD_INITIAL_CACHE);
+ client.newNamespaceAwareEnsurePath("/test/aaa").ensure(client.getZookeeperClient());
+ client.setData().forPath("/test/aaa", new byte[]{1, 2, 3, 4, 5});
+ cache.rebuildNode("/test/aaa");
+ CloseableUtils.closeQuietly(cache);
+ } finally {
+ CloseableUtils.closeQuietly(client);
+ }
+ }
+
+ Assert.assertEquals(messages.size(), 1, "There should not be any error events except for the test message, " +
+ "but got:\n" + Joiner.on("\n").join(messages));
+
+ }
+
+ @Test
public void testAsyncInitialPopulation() throws Exception
{
Timing timing = new Timing();
http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 2129e56..635819b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -279,6 +279,12 @@
</dependency>
<dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <version>1.7.6</version>
+ </dependency>
+
+ <dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>1.9.5</version>
[4/4] git commit: CURATOR-141 - Reverted the logging additions to the
pom.xml as they have already been covered by CURATOR-139
Posted by ca...@apache.org.
CURATOR-141 - Reverted the logging additions to the pom.xml as they have already been covered by CURATOR-139
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/0d1397af
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/0d1397af
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/0d1397af
Branch: refs/heads/CURATOR-141
Commit: 0d1397af156f1e8214b668d74583d4fb3715ffad
Parents: c623053
Author: Cam McKenzie <ca...@apache.org>
Authored: Wed Aug 20 15:54:06 2014 +1000
Committer: Cam McKenzie <ca...@apache.org>
Committed: Wed Aug 20 15:54:06 2014 +1000
----------------------------------------------------------------------
curator-recipes/pom.xml | 6 ------
pom.xml | 6 ------
2 files changed, 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/0d1397af/curator-recipes/pom.xml
----------------------------------------------------------------------
diff --git a/curator-recipes/pom.xml b/curator-recipes/pom.xml
index 37155c9..13aa475 100644
--- a/curator-recipes/pom.xml
+++ b/curator-recipes/pom.xml
@@ -61,11 +61,5 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
-
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-log4j12</artifactId>
- <scope>test</scope>
- </dependency>
</dependencies>
</project>
http://git-wip-us.apache.org/repos/asf/curator/blob/0d1397af/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 6840de7..60e718c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -286,12 +286,6 @@
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-log4j12</artifactId>
- <version>1.7.6</version>
- </dependency>
-
- <dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>1.9.5</version>
[2/4] git commit: CURATOR-141 Changed the way we detect a working log
setup
Posted by ca...@apache.org.
CURATOR-141 Changed the way we detect a working log setup
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/4f9d27a2
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/4f9d27a2
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/4f9d27a2
Branch: refs/heads/CURATOR-141
Commit: 4f9d27a2af09ce8e6825bb489ec7e77597ad178a
Parents: f2f1953
Author: Karel Vervaeke <ka...@ngdata.com>
Authored: Mon Aug 18 14:25:39 2014 +0200
Committer: Karel Vervaeke <ka...@ngdata.com>
Committed: Mon Aug 18 14:25:39 2014 +0200
----------------------------------------------------------------------
.../recipes/cache/TestPathChildrenCache.java | 32 ++++++++++++++++----
1 file changed, 26 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/4f9d27a2/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
index 70156ff..b904bdc 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
@@ -24,6 +24,9 @@ import com.google.common.collect.Collections2;
import com.google.common.collect.Lists;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.api.BackgroundCallback;
+import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.api.Pathable;
import org.apache.curator.framework.api.UnhandledErrorListener;
import org.apache.curator.framework.imps.CuratorFrameworkImpl;
import org.apache.curator.retry.RetryOneTime;
@@ -129,13 +132,30 @@ public class TestPathChildrenCache extends BaseClassForTests
}
};
appender.setLayout(new SimpleLayout());
- Logger rootLogger = Logger.getLogger("org.apache.curator");
- rootLogger.addAppender(appender);
+ Logger logger = Logger.getLogger("org.apache.curator");
+ logger.addAppender(appender);
+
+ // Check that we can intercept error log messages from the client
+ CuratorFramework clientTestLogSetup = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ clientTestLogSetup.start();
+ try {
+ Pathable<byte[]> callback = clientTestLogSetup.getData().inBackground(new BackgroundCallback() {
+ @Override
+ public void processResult(CuratorFramework client, CuratorEvent event) throws Exception {
+ // ignore result
+ }
+ });
+ CloseableUtils.closeQuietly(clientTestLogSetup);
+ callback.forPath("/test/aaa"); // this should cause an error log message
+ } catch (IllegalStateException ise) {
+ // ok, excpected
+ } finally {
+ CloseableUtils.closeQuietly(clientTestLogSetup);
+ }
- // Check that the logging setup didn't change in a way that breaks this test
- Logger.getLogger(CuratorFrameworkImpl.class).error("Just checking");
- Assert.assertTrue(messages.contains("Just checking"),
- "The test error message was not logged, this test may be broken");
+ Assert.assertTrue(messages.contains("Background exception was not retry-able or retry gave up"),
+ "The expected error was not logged. This is an indication that this test could be broken due to" +
+ " an incomplete logging setup.");
// try to reproduce a bunch of times because it doesn't happen reliably
for (int i = 0; i < 50; i++) {
[3/4] git commit: Merge branch 'CURATOR-141' of
https://github.com/karel1980/curator into CURATOR-141
Posted by ca...@apache.org.
Merge branch 'CURATOR-141' of https://github.com/karel1980/curator into CURATOR-141
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/c6230536
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/c6230536
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/c6230536
Branch: refs/heads/CURATOR-141
Commit: c62305365c8a4efd19108cd5f23017c7d9b62fc6
Parents: 104837d 4f9d27a
Author: Cam McKenzie <ca...@apache.org>
Authored: Wed Aug 20 12:58:12 2014 +1000
Committer: Cam McKenzie <ca...@apache.org>
Committed: Wed Aug 20 12:58:12 2014 +1000
----------------------------------------------------------------------
curator-recipes/pom.xml | 6 ++
.../recipes/cache/PathChildrenCache.java | 4 +
.../recipes/cache/TestPathChildrenCache.java | 105 ++++++++++++++++++-
pom.xml | 6 ++
4 files changed, 120 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/c6230536/pom.xml
----------------------------------------------------------------------