zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fang...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
Date Thu, 20 Dec 2018 14:08:29 GMT
This is an automated email from the ASF dual-hosted git repository.

fangmin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new b8c8937  ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
b8c8937 is described below

commit b8c8937029ca7e15116e91c6a6c436368ac0f9bb
Author: Ilya Maykov <ilyam@fb.com>
AuthorDate: Thu Dec 20 22:07:14 2018 +0800

    ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
    
    Made some changes to handle the random ENTRY_CREATE events that occasionally fire even
though the watcher is created after the file is already written to disk. Should make the test
more stable.
    
    Author: Ilya Maykov <ilyam@fb.com>
    
    Closes #739 from ivmaykov/ZOOKEEPER-3219
---
 .../zookeeper/common/FileChangeWatcherTest.java    | 46 ++++++++++++++--------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
index e4950f3..2ef6a86 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
@@ -38,6 +38,7 @@ import org.slf4j.LoggerFactory;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class FileChangeWatcherTest extends ZKTestCase {
     private static File tempDir;
@@ -70,6 +71,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
                     tempDir.toPath(),
                     event -> {
                         LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
                         synchronized (events) {
                             events.add(event);
                             events.notifyAll();
@@ -112,6 +119,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
                     tempDir.toPath(),
                     event -> {
                         LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
                         synchronized (events) {
                             events.add(event);
                             events.notifyAll();
@@ -184,6 +197,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
                     tempDir.toPath(),
                     event -> {
                         LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
                         synchronized (events) {
                             events.add(event);
                             events.notifyAll();
@@ -214,21 +233,18 @@ public class FileChangeWatcherTest extends ZKTestCase {
     public void testCallbackErrorDoesNotCrashWatcherThread() throws IOException, InterruptedException
{
         FileChangeWatcher watcher = null;
         try {
-            final List<WatchEvent<?>> events = new ArrayList<>();
             final AtomicInteger callCount = new AtomicInteger(0);
             watcher = new FileChangeWatcher(
                     tempDir.toPath(),
                     event -> {
                         LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        int oldValue;
                         synchronized (callCount) {
-                            if (callCount.getAndIncrement() == 0) {
-                                callCount.notifyAll();
-                                throw new RuntimeException("This error should not crash the
watcher thread");
-                            }
+                            oldValue = callCount.getAndIncrement();
+                            callCount.notifyAll();
                         }
-                        synchronized (events) {
-                            events.add(event);
-                            events.notifyAll();
+                        if (oldValue == 0) {
+                            throw new RuntimeException("This error should not crash the watcher
thread");
                         }
                     });
             watcher.start();
@@ -243,16 +259,14 @@ public class FileChangeWatcherTest extends ZKTestCase {
             }
             LOG.info("Modifying file again");
             FileUtils.writeStringToFile(tempFile, "Hello world again\n", StandardCharsets.UTF_8,
true);
-            synchronized (events) {
-                if (events.isEmpty()) {
-                    events.wait(3000L);
+            synchronized (callCount) {
+                if (callCount.get() == 1) {
+                    callCount.wait(3000L);
                 }
-                assertEquals(2, callCount.get());
-                assertFalse(events.isEmpty());
-                WatchEvent<?> event = events.get(0);
-                assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
-                assertEquals(tempFile.getName(), event.context().toString());
             }
+            // The value of callCount can exceed 1 only if the callback thread
+            // survives the exception thrown by the first callback.
+            assertTrue(callCount.get() > 1);
         } finally {
             if (watcher != null) {
                 watcher.stop();


Mime
View raw message