You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ak...@apache.org on 2017/04/01 00:14:45 UTC

sentry git commit: SENTRY-1683: MetastoreCacheInitializer has a race condition in handling results list (Alex Kolbasov, Reviewed by: Hao Hao)

Repository: sentry
Updated Branches:
  refs/heads/master a913b9d0e -> 1992e5bde


SENTRY-1683: MetastoreCacheInitializer has a race condition in handling results list (Alex Kolbasov, Reviewed by: Hao Hao)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/1992e5bd
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/1992e5bd
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/1992e5bd

Branch: refs/heads/master
Commit: 1992e5bdebefe019820c2a2219efe32e63fd44ec
Parents: a913b9d
Author: Alexander Kolbasov <ak...@cloudera.com>
Authored: Fri Mar 31 17:14:22 2017 -0700
Committer: Alexander Kolbasov <ak...@cloudera.com>
Committed: Fri Mar 31 17:14:22 2017 -0700

----------------------------------------------------------------------
 .../sentry/hdfs/MetastoreCacheInitializer.java  | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/1992e5bd/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
index f9664f0..ac8d8f4 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
@@ -32,7 +32,9 @@ import org.slf4j.LoggerFactory;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Vector;
 import java.util.concurrent.*;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -248,9 +250,7 @@ class MetastoreCacheInitializer implements Closeable {
             Callable<CallResult> partTask =
                     new PartitionTask(db.getName(), tableName,
                             partsToFetch, tblPathChange);
-            synchronized (results) {
-              results.add(threadPool.submit(partTask));
-            }
+            results.add(threadPool.submit(partTask));
           }
         }
       }
@@ -284,9 +284,9 @@ class MetastoreCacheInitializer implements Closeable {
         throw new SentryMalformedPathException(msg, e);
       }
       if (dbPath != null) {
+        Preconditions.checkArgument(dbName.equalsIgnoreCase(db.getName()),
+                "Inconsistent database names \"%s\" vs \"%s\"", dbName, db.getName());
         synchronized (update) {
-          Preconditions.checkArgument(dbName.equalsIgnoreCase(db.getName()),
-            "Inconsistent database names \"%s\" vs \"%s\"", dbName, db.getName());
           update.newPathChange(dbName).addToAddPaths(dbPath);
         }
       }
@@ -298,9 +298,7 @@ class MetastoreCacheInitializer implements Closeable {
                         i + maxTablesPerCall, allTblStr.size()));
         Callable<CallResult> tableTask =
                 new TableTask(db, tablesToFetch, update);
-        synchronized (results) {
           results.add(threadPool.submit(tableTask));
-        }
       }
     }
   }
@@ -309,8 +307,8 @@ class MetastoreCacheInitializer implements Closeable {
   private final IHMSHandler hmsHandler;
   private final int maxPartitionsPerCall;
   private final int maxTablesPerCall;
-  private final List<Future<CallResult>> results =
-          new ArrayList<Future<CallResult>>();
+  // We use Vector because it is thread-safe
+  private final Collection<Future<CallResult>> results = new Vector<>();
   private final AtomicInteger taskCounter = new AtomicInteger(0);
   private final int maxRetries;
   private final int waitDurationMillis;
@@ -361,8 +359,8 @@ class MetastoreCacheInitializer implements Closeable {
       results.add(threadPool.submit(dbTask));
     }
 
-    while (taskCounter.get() > 0) {
-      Thread.sleep(1000);
+    while (taskCounter.get() != 0) {
+      Thread.sleep(250);
       // Wait until no more tasks remain
     }