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 <[email protected]> Authored: Fri Mar 31 17:14:22 2017 -0700 Committer: Alexander Kolbasov <[email protected]> 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 }
