Repository: incubator-sentry Updated Branches: refs/heads/master 8624435bc -> 9fe720232
SENTRY-957: Exceptions in MetastoreCacheInitializer should probably not prevent HMS from starting up (Hao Hao via Lenni Kuff) Change-Id: I13207ed65366b2b22a6f21de5dc9888b50f96091 Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/9fe72023 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/9fe72023 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/9fe72023 Branch: refs/heads/master Commit: 9fe720232120b27ada196a7693e57c2127db80ee Parents: 8624435 Author: Lenni Kuff <[email protected]> Authored: Sun Dec 13 00:12:17 2015 -0800 Committer: Lenni Kuff <[email protected]> Committed: Sun Dec 13 00:12:17 2015 -0800 ---------------------------------------------------------------------- .../apache/sentry/hdfs/ServiceConstants.java | 6 + .../sentry/hdfs/MetastoreCacheInitializer.java | 125 ++++++++++++++++--- .../hdfs/TestMetastoreCacheInitializer.java | 2 + 3 files changed, 115 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9fe72023/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java index 8f62496..1fdf418 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java +++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java @@ -53,6 +53,12 @@ public class ServiceConstants { public static final String SENTRY_METASTORE_HA_ZOOKEEPER_NAMESPACE_DEFAULT = "/sentry_metastore"; public static final String SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS = "sentry.hdfs.sync.metastore.cache.init.threads"; public static final int SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS_DEFAULT = 10; + public static final String SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM = "sentry.hdfs.sync.metastore.cache.retry.max.num"; + public static final int SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM_DEFAULT = 1; + public static final String SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_WAIT_DURAION_IN_MILLIS = "sentry.hdfs.sync.metastore.cache.retry.wait.duration.millis"; + public static final int SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_WAIT_DURAION_IN_MILLIS_DEFAULT = 1000; + public static final String SENTRY_HDFS_SYNC_METASTORE_CACHE_FAIL_ON_PARTIAL_UPDATE = "sentry.hdfs.sync.metastore.cache.fail.on.partial.update"; + public static final boolean SENTRY_HDFS_SYNC_METASTORE_CACHE_FAIL_ON_PARTIAL_UPDATE_DEFAULT = true; public static final String SENTRY_HDFS_SYNC_METASTORE_CACHE_ASYNC_INIT_ENABLE = "sentry.hdfs.sync.metastore.cache.async-init.enable"; public static final boolean SENTRY_HDFS_SYNC_METASTORE_CACHE_ASYNC_INIT_ENABLE_DEFAULT = false; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9fe72023/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 eb85d45..4349c6e 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 @@ -42,32 +42,98 @@ class MetastoreCacheInitializer implements Closeable { private static final Logger LOGGER = LoggerFactory.getLogger (MetastoreCacheInitializer.class); - static class CallResult { - final Exception failure; + final static class CallResult { + final private Exception failure; + final private boolean successStatus; - CallResult(Exception ex) { + CallResult(Exception ex, boolean successStatus) { failure = ex; + this.successStatus = successStatus; + } + + public boolean getSuccessStatus() { + return successStatus; + } + + public Exception getFailure() { + return failure; } } abstract class BaseTask implements Callable<CallResult> { - BaseTask() { taskCounter.incrementAndGet(); } + /** + * Class represents retry strategy for BaseTask. + */ + private class RetryStrategy { + private int maxRetries = 0; + private int waitDurationMillis; + private int retries; + private Exception exception; + + private RetryStrategy(int maxRetries, int waitDurationMillis) { + this.maxRetries = maxRetries; + retries = 0; + + // Assign default wait duration if negative value is provided. + if (waitDurationMillis > 0) { + this.waitDurationMillis = waitDurationMillis; + } else { + this.waitDurationMillis = 1000; + } + } + + public CallResult exec() { + + // Retry logic is happening inside callable/task to avoid + // synchronous waiting on getting the result. + // Retry the failure task until reach the max retry number. + // Wait configurable duration for next retry. + for (int i = 0; i < maxRetries; i++) { + try { + doTask(); + + // Task succeeds, reset the exception and return + // the successful flag. + exception = null; + return new CallResult(exception, true); + } catch (Exception ex) { + LOGGER.debug("Failed to execute task on " + (i + 1) + " attempts." + + " Sleeping for " + waitDurationMillis + " ms. Exception: " + ex.toString(), ex); + exception = ex; + + try { + Thread.sleep(waitDurationMillis); + } catch (InterruptedException exception) { + // Skip the rest retries if get InterruptedException. + // And set the corresponding retries number. + retries = i; + i = maxRetries; + } + } + + retries = i; + } + + // Task fails, return the failure flag. + LOGGER.error("Task did not complete successfully after " + retries + + " tries. Exception got: " + exception.toString()); + return new CallResult(exception, false); + } + } + + private RetryStrategy retryStrategy; + + BaseTask() { + taskCounter.incrementAndGet(); + retryStrategy = new RetryStrategy(maxRetries, waitDurationMillis); + } @Override public CallResult call() throws Exception { - Exception e = null; - try { - doTask(); - } catch (Exception ex) { - // Ignore if object requested does not exists - if (!(ex instanceof NoSuchObjectException) ){ - e = ex; - } - } finally { - taskCounter.decrementAndGet(); - } - return new CallResult(e); + CallResult callResult = retryStrategy.exec(); + taskCounter.decrementAndGet(); + return callResult; } abstract void doTask() throws Exception; @@ -201,6 +267,9 @@ class MetastoreCacheInitializer implements Closeable { private final List<Future<CallResult>> results = new ArrayList<Future<CallResult>>(); private final AtomicInteger taskCounter = new AtomicInteger(0); + private final int maxRetries; + private final int waitDurationMillis; + private final boolean failOnRetry; MetastoreCacheInitializer(IHMSHandler hmsHandler, Configuration conf) { this.hmsHandler = hmsHandler; @@ -219,6 +288,21 @@ class MetastoreCacheInitializer implements Closeable { .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS, ServiceConstants.ServerConfig .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS_DEFAULT)); + maxRetries = conf.getInt( + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM, + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM_DEFAULT); + waitDurationMillis = conf.getInt( + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_WAIT_DURAION_IN_MILLIS, + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_WAIT_DURAION_IN_MILLIS_DEFAULT); + failOnRetry = conf.getBoolean( + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_FAIL_ON_PARTIAL_UPDATE, + ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_FAIL_ON_PARTIAL_UPDATE_DEFAULT); } UpdateableAuthzPaths createInitialUpdate() throws @@ -236,12 +320,17 @@ class MetastoreCacheInitializer implements Closeable { Thread.sleep(1000); // Wait until no more tasks remain } + for (Future<CallResult> result : results) { CallResult callResult = result.get(); - if (callResult.failure != null) { - throw new RuntimeException(callResult.failure); + + // Fail the HMS startup if tasks are not all successful and + // fail on partial updates flag is set in the config. + if (callResult.getSuccessStatus() == false && failOnRetry) { + throw new RuntimeException(callResult.getFailure()); } } + authzPaths.updatePartial(Lists.newArrayList(tempUpdate), new ReentrantReadWriteLock()); return authzPaths; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9fe72023/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java index f1e729f..437ba94 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java +++ b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java @@ -161,6 +161,8 @@ public class TestMetastoreCacheInitializer { .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_TABLES_PER_RPC, 1); conf.setInt(ServiceConstants.ServerConfig .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS, 1); + conf.setInt(ServiceConstants.ServerConfig + .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM, 2); try { MetastoreCacheInitializer cacheInitializer = new
