You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@asterixdb.apache.org by "Xikui Wang (Code Review)" <do...@asterixdb.incubator.apache.org> on 2016/12/17 04:05:35 UTC

Change in asterixdb[master]: Fix ASTERIX-1690

Xikui Wang has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/1393

Change subject: Fix ASTERIX-1690
......................................................................

Fix ASTERIX-1690

Fix the deadlock problem ASTERIX-1690 in FileSystemWatcher

Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
---
M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
3 files changed, 37 insertions(+), 35 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/93/1393/1

diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
index ea5cc8f..96c1e31 100644
--- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
+++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
@@ -34,7 +34,6 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.log4j.Level;
@@ -53,7 +52,6 @@
     private final boolean isFeed;
     private boolean done;
     private final LinkedList<Path> dirs;
-    private final ReentrantLock lock = new ReentrantLock();
 
     public FileSystemWatcher(List<Path> inputResources, String expression, boolean isFeed) throws HyracksDataException {
         this.isFeed = isFeed;
@@ -142,7 +140,7 @@
         return (WatchEvent<T>) event;
     }
 
-    private void handleEvents(WatchKey key) throws IOException {
+    private synchronized void handleEvents(WatchKey key) throws IOException {
         // get dir associated with the key
         Path dir = keys.get(key);
         if (dir == null) {
@@ -169,7 +167,7 @@
             Path name = ev.context();
             Path child = dir.resolve(name);
             // if directory is created then register it and its sub-directories
-            if ((kind == StandardWatchEventKinds.ENTRY_CREATE)) {
+            if (kind == StandardWatchEventKinds.ENTRY_CREATE) {
                 try {
                     if (Files.isDirectory(child, LinkOption.NOFOLLOW_LINKS)) {
                         register(child);
@@ -226,7 +224,7 @@
     }
 
     // take is blocking
-    public synchronized File take() throws IOException {
+    public File take() throws IOException {
         File next = poll();
         if (next != null) {
             return next;
@@ -235,36 +233,31 @@
             return null;
         }
         // No file was found, wait for the filesystem to push events
-        WatchKey key = null;
-        lock.lock();
-        try {
-            while (!it.hasNext()) {
-                try {
-                    key = watcher.take();
-                } catch (InterruptedException x) {
-                    if (LOGGER.isEnabledFor(Level.WARN)) {
-                        LOGGER.warn("Feed Closed");
-                    }
-                    if (watcher == null) {
-                        return null;
-                    }
-                    continue;
-                } catch (ClosedWatchServiceException e) {
-                    if (LOGGER.isEnabledFor(Level.WARN)) {
-                        LOGGER.warn("The watcher has exited");
-                    }
-                    if (watcher == null) {
-                        return null;
-                    }
-                    continue;
+        WatchKey key;
+        while (!it.hasNext()) {
+            try {
+                key = watcher.take();
+            } catch (InterruptedException x) {
+                if (LOGGER.isEnabledFor(Level.WARN)) {
+                    LOGGER.warn("Feed Closed");
                 }
-                handleEvents(key);
-                if (endOfEvents(key)) {
+                if (watcher == null) {
                     return null;
                 }
+                continue;
+            } catch (ClosedWatchServiceException e) {
+                if (LOGGER.isEnabledFor(Level.WARN)) {
+                    LOGGER.warn("The watcher has exited");
+                }
+                if (watcher == null) {
+                    return null;
+                }
+                continue;
             }
-        } finally {
-            lock.unlock();
+            handleEvents(key);
+            if (endOfEvents(key)) {
+                return null;
+            }
         }
         // files were found, re-create the iterator and move it one step
         return it.next();
diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
index 16dd1e9..6553e47 100644
--- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
+++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
@@ -27,13 +27,14 @@
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.regex.Pattern;
 
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 
 public class LocalFileSystemUtils {
 
-    public static void traverse(final LinkedList<File> files, File root, final String expression,
+    public static void traverse(final List<File> files, File root, final String expression,
             final LinkedList<Path> dirs) throws IOException {
         final Path path = root.toPath();
         if (!Files.exists(path)) {
@@ -68,8 +69,17 @@
         });
     }
 
-    public static void validateAndAdd(Path path, String expression, LinkedList<File> files) {
-        if (expression == null || Pattern.matches(expression, path.toString())) {
+    private static boolean fileNotExistsInList(List<File> files, Path path) {
+        for (File file : files) {
+            if (file.getPath().equals(path.toString())) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    public static void validateAndAdd(Path path, String expression, List<File> files) {
+        if ((expression == null || Pattern.matches(expression, path.toString())) && fileNotExistsInList(files, path)) {
             files.add(new File(path.toString()));
         }
     }
diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
index 12e854d..1afd196 100644
--- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
+++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
@@ -323,7 +323,6 @@
             ARecordType adapterOutputType = getOutputType(feed, configuration, ExternalDataConstants.KEY_TYPE_NAME);
             ARecordType metaType = getOutputType(feed, configuration, ExternalDataConstants.KEY_META_TYPE_NAME);
             ExternalDataUtils.prepareFeed(configuration, feed.getDataverseName(), feed.getFeedName());
-            ExternalDataUtils.prepareFeed(configuration, feed.getDataverseName(), feed.getFeedName());
             // Get adapter from metadata dataset <Metadata dataverse>
             DatasourceAdapter adapterEntity = MetadataManager.INSTANCE.getAdapter(mdTxnCtx,
                     MetadataConstants.METADATA_DATAVERSE_NAME, adapterName);

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>

Change in asterixdb[master]: Fix ASTERIX-1690

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.

Jenkins has posted comments on this change.

Change subject: Fix ASTERIX-1690
......................................................................


Patch Set 1: BAD+1

BAD Compatibility Tests Successful

https://asterix-jenkins.ics.uci.edu/job/asterixbad-compat/61/ : SUCCESS

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix ASTERIXDB-1690

Posted by "Xikui Wang (Code Review)" <do...@asterixdb.incubator.apache.org>.

Xikui Wang has submitted this change and it was merged.

Change subject: Fix ASTERIXDB-1690
......................................................................


Fix ASTERIXDB-1690

Fix the deadlock problem ASTERIXDB-1690 in FileSystemWatcher

Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1393
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
BAD: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: abdullah alamoudi <ba...@gmail.com>
---
M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
3 files changed, 37 insertions(+), 35 deletions(-)

Approvals:
  abdullah alamoudi: Looks good to me, approved
  Jenkins: Verified; No violations found; Verified

Objections:
  Jenkins: Violations found



diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
index 258f194..ab1c424 100644
--- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
+++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
@@ -34,7 +34,6 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.asterix.common.exceptions.ErrorCode;
 import org.apache.asterix.common.exceptions.RuntimeDataException;
@@ -55,7 +54,6 @@
     private final boolean isFeed;
     private boolean done;
     private final LinkedList<Path> dirs;
-    private final ReentrantLock lock = new ReentrantLock();
 
     public FileSystemWatcher(List<Path> inputResources, String expression, boolean isFeed) throws HyracksDataException {
         this.isFeed = isFeed;
@@ -145,7 +143,7 @@
         return (WatchEvent<T>) event;
     }
 
-    private void handleEvents(WatchKey key) throws IOException {
+    private synchronized void handleEvents(WatchKey key) throws IOException {
         // get dir associated with the key
         Path dir = keys.get(key);
         if (dir == null) {
@@ -172,7 +170,7 @@
             Path name = ev.context();
             Path child = dir.resolve(name);
             // if directory is created then register it and its sub-directories
-            if ((kind == StandardWatchEventKinds.ENTRY_CREATE)) {
+            if (kind == StandardWatchEventKinds.ENTRY_CREATE) {
                 try {
                     if (Files.isDirectory(child, LinkOption.NOFOLLOW_LINKS)) {
                         register(child);
@@ -229,7 +227,7 @@
     }
 
     // take is blocking
-    public synchronized File take() throws IOException {
+    public File take() throws IOException {
         File next = poll();
         if (next != null) {
             return next;
@@ -238,36 +236,31 @@
             return null;
         }
         // No file was found, wait for the filesystem to push events
-        WatchKey key = null;
-        lock.lock();
-        try {
-            while (!it.hasNext()) {
-                try {
-                    key = watcher.take();
-                } catch (InterruptedException x) {
-                    if (LOGGER.isEnabledFor(Level.WARN)) {
-                        LOGGER.warn("Feed Closed");
-                    }
-                    if (watcher == null) {
-                        return null;
-                    }
-                    continue;
-                } catch (ClosedWatchServiceException e) {
-                    if (LOGGER.isEnabledFor(Level.WARN)) {
-                        LOGGER.warn("The watcher has exited");
-                    }
-                    if (watcher == null) {
-                        return null;
-                    }
-                    continue;
+        WatchKey key;
+        while (!it.hasNext()) {
+            try {
+                key = watcher.take();
+            } catch (InterruptedException x) {
+                if (LOGGER.isEnabledFor(Level.WARN)) {
+                    LOGGER.warn("Feed Closed");
                 }
-                handleEvents(key);
-                if (endOfEvents(key)) {
+                if (watcher == null) {
                     return null;
                 }
+                continue;
+            } catch (ClosedWatchServiceException e) {
+                if (LOGGER.isEnabledFor(Level.WARN)) {
+                    LOGGER.warn("The watcher has exited");
+                }
+                if (watcher == null) {
+                    return null;
+                }
+                continue;
             }
-        } finally {
-            lock.unlock();
+            handleEvents(key);
+            if (endOfEvents(key)) {
+                return null;
+            }
         }
         // files were found, re-create the iterator and move it one step
         return it.next();
diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
index f6046ef..2cb842b 100644
--- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
+++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
@@ -27,6 +27,7 @@
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.regex.Pattern;
 
 import org.apache.asterix.common.exceptions.ErrorCode;
@@ -34,7 +35,7 @@
 
 public class LocalFileSystemUtils {
 
-    public static void traverse(final LinkedList<File> files, File root, final String expression,
+    public static void traverse(final List<File> files, File root, final String expression,
             final LinkedList<Path> dirs) throws IOException {
         final Path path = root.toPath();
         if (!Files.exists(path)) {
@@ -70,8 +71,17 @@
         });
     }
 
-    public static void validateAndAdd(Path path, String expression, LinkedList<File> files) {
-        if (expression == null || Pattern.matches(expression, path.toString())) {
+    private static boolean fileNotExistsInList(List<File> files, Path path) {
+        for (File file : files) {
+            if (file.getPath().equals(path.toString())) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    public static void validateAndAdd(Path path, String expression, List<File> files) {
+        if ((expression == null || Pattern.matches(expression, path.toString())) && fileNotExistsInList(files, path)) {
             files.add(new File(path.toString()));
         }
     }
diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
index 584aead..b54c9e6 100644
--- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
+++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java
@@ -103,7 +103,6 @@
             ARecordType adapterOutputType = getOutputType(feed, configuration, ExternalDataConstants.KEY_TYPE_NAME);
             ARecordType metaType = getOutputType(feed, configuration, ExternalDataConstants.KEY_META_TYPE_NAME);
             ExternalDataUtils.prepareFeed(configuration, feed.getDataverseName(), feed.getFeedName());
-            ExternalDataUtils.prepareFeed(configuration, feed.getDataverseName(), feed.getFeedName());
             // Get adapter from metadata dataset <Metadata dataverse>
             DatasourceAdapter adapterEntity = MetadataManager.INSTANCE.getAdapter(mdTxnCtx,
                     MetadataConstants.METADATA_DATAVERSE_NAME, adapterName);

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>

Change in asterixdb[master]: Fix ASTERIXDB-1690

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.

Jenkins has posted comments on this change.

Change subject: Fix ASTERIXDB-1690
......................................................................


Patch Set 4:

Build Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-verify-no-installer-app/138/ (6/6)

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xk...@gmail.com>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix ASTERIXDB-1690

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.

Jenkins has posted comments on this change.

Change subject: Fix ASTERIXDB-1690
......................................................................


Patch Set 3:

Build Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-notopic/4967/ (6/6)

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix ASTERIXDB-1690

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.

Jenkins has posted comments on this change.

Change subject: Fix ASTERIXDB-1690
......................................................................


Patch Set 4:

BAD Compatibility Tests Started https://asterix-jenkins.ics.uci.edu/job/asterixbad-compat/1008/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xk...@gmail.com>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix ASTERIXDB-1690

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.

Jenkins has posted comments on this change.

Change subject: Fix ASTERIXDB-1690
......................................................................


Patch Set 4:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/2487/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1393
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad358fdeeb47f5d5884fed8806a234f8b3196bec
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xk...@gmail.com>
Gerrit-HasComments: No