sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lsk...@apache.org
Subject incubator-sentry git commit: SENTRY-957: Exceptions in MetastoreCacheInitializer should probably not prevent HMS from starting up (Hao Hao via Lenni Kuff)
Date Sun, 13 Dec 2015 08:12:41 GMT
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 <lskuff@cloudera.com>
Authored: Sun Dec 13 00:12:17 2015 -0800
Committer: Lenni Kuff <lskuff@cloudera.com>
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


Mime
View raw message