savepoint name

Author: dengzhhu653

standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java

@@ -1309,6 +1309,14 @@ public enum ConfVars {
             + "  seqprefix: adds a 'N_' prefix to the table name to get a unique location (table,1_table,2_table,...)\n"
             + "  prohibit: do not consider alternate locations; throw error if the default is not available\n"
             + "  force: use the default location even in case the directory is already available"),
+    METASTORE_S4U_NOWAIT_MAX_RETRIES("metastore.s4u.nowait.max.retries",
+        "hive.metastore.s4u.nowait.max.retries", 20,
+        "Number of retries required to acquire a row lock immediately without waiting."),
+    METASTORE_S4U_NOWAIT_RETRY_SLEEP_INTERVAL(
+        "metastore.s4u.nowait.retry.sleep.interval",
+        "hive.metastore.s4u.nowait.retry.sleep.interval", 300, TimeUnit.MILLISECONDS,
+        "Sleep interval between retries to acquire a row lock immediately described part of property "
+            + METASTORE_S4U_NOWAIT_MAX_RETRIES.name()),
 
     MULTITHREADED("javax.jdo.option.Multithreaded", "javax.jdo.option.Multithreaded", true,
         "Set this to true if multiple threads access metastore through JDO concurrently."),

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java

@@ -476,7 +476,7 @@ public String addForUpdateClause(String selectStatement, boolean noWait) throws
     case SQLSERVER:
       //https://msdn.microsoft.com/en-us/library/ms189499.aspx
       //https://msdn.microsoft.com/en-us/library/ms187373.aspx
-      String modifier = " with (updlock " + (noWait ? ", NOWAIT" : "") + ")";
+      String modifier = " with (updlock" + (noWait ? ",NOWAIT" : "") + ")";
       int wherePos = selectStatement.toUpperCase().indexOf(" WHERE ");
       if (wherePos < 0) {
         return selectStatement + modifier;

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java

@@ -53,6 +53,7 @@
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.UUID;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
@@ -9468,17 +9469,20 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
     boolean committed = false;
     List<ColumnStatisticsObj> statsObjs = colStats.getStatsObj();
     ColumnStatisticsDesc statsDesc = colStats.getStatsDesc();
-    
+    long start = System.currentTimeMillis();
+    String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
     try {
       openTransaction();
       // DataNucleus objects get detached all over the place for no (real) reason.
       // So let's not use them anywhere unless absolutely necessary.
-      String catName = statsDesc.isSetCatName() ? statsDesc.getCatName() : getDefaultCatalog(conf);
+      int maxRetries = MetastoreConf.getIntVar(conf, ConfVars.METASTORE_S4U_NOWAIT_MAX_RETRIES);
+      long sleepInterval = ThreadLocalRandom.current().nextLong(MetastoreConf.getTimeVar(conf,
+          ConfVars.METASTORE_S4U_NOWAIT_RETRY_SLEEP_INTERVAL, TimeUnit.MILLISECONDS)) + 30;
       MTable mTable = ensureGetMTable(catName, statsDesc.getDbName(), statsDesc.getTableName());
       Ref<Map<String, String>> result = new Ref<>();
-      new RetryingExecutor(conf, () -> {
+      new RetryingExecutor(maxRetries, sleepInterval, () -> {
         Ref<Exception> exceptionRef = new Ref<>();
-        String savePoint = "ObjectStore:updateTableColumnStatistics:" + System.nanoTime();
+        String savePoint = "uts_" + ThreadLocalRandom.current().nextInt(10000) + "_" + System.nanoTime();
         setTransactionSavePoint(savePoint);
         executePlainSQL(
             sqlGenerator.addForUpdateNoWait("SELECT \"TBL_ID\" FROM \"TBLS\" WHERE \"TBL_ID\" = " + mTable.getId()),
@@ -9542,6 +9546,9 @@ public Map<String, String> updateTableColumnStatistics(ColumnStatistics colStats
       // TODO: similar to update...Part, this used to do "return committed;"; makes little sense.
       return committed ? result.t : null;
     } finally {
+      LOG.info("{} updateTableColumnStatistics took {}ms, success: {}",
+          new TableName(catName, statsDesc.getDbName(), statsDesc.getTableName()),
+          System.currentTimeMillis() - start, committed);
       rollbackAndCleanup(committed, null);
     }
   }
@@ -11441,21 +11448,21 @@ private void executePlainSQL(String sql, Consumer<Exception> exceptionConsumer)
   }
 
   private void lockNotificationSequenceForUpdate() throws MetaException {
-    long maxRetries =
+    int maxRetries =
         MetastoreConf.getIntVar(conf, ConfVars.NOTIFICATION_SEQUENCE_LOCK_MAX_RETRIES);
     long sleepInterval = MetastoreConf.getTimeVar(conf,
         ConfVars.NOTIFICATION_SEQUENCE_LOCK_RETRY_SLEEP_INTERVAL, TimeUnit.MILLISECONDS);
     if (sqlGenerator.getDbProduct().isDERBY() && directSql != null) {
       // Derby doesn't allow FOR UPDATE to lock the row being selected (See https://db.apache
       // .org/derby/docs/10.1/ref/rrefsqlj31783.html) . So lock the whole table. Since there's
       // only one row in the table, this shouldn't cause any performance degradation.
-      new RetryingExecutor((int) maxRetries, sleepInterval, () -> {
+      new RetryingExecutor(maxRetries, sleepInterval, () -> {
         directSql.lockDbTable("NOTIFICATION_SEQUENCE");
       }).commandName("lockNotificationSequenceForUpdate").run();
     } else {
       String selectQuery = "select \"NEXT_EVENT_ID\" from \"NOTIFICATION_SEQUENCE\"";
       String lockingQuery = sqlGenerator.addForUpdateClause(selectQuery);
-      new RetryingExecutor((int) maxRetries, sleepInterval, () -> executePlainSQL(lockingQuery, null))
+      new RetryingExecutor(maxRetries, sleepInterval, () -> executePlainSQL(lockingQuery, null))
           .commandName("lockNotificationSequenceForUpdate").run();
     }
   }

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/RetryingExecutor.java

@@ -20,11 +20,8 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hive.metastore.api.MetaException;
-import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -36,9 +33,9 @@ public interface Command {
   private static Logger LOG = LoggerFactory.getLogger(RetryingExecutor.class);
   private final int maxRetries;
   private final long sleepInterval;
-  private int currentRetries = 0;
   private final Command command;
-  private List<Class<? extends Exception>> retriableException = new ArrayList<>();
+  private final List<Class<? extends Exception>> retriableException = new ArrayList<>();
+  private int currentRetries = 0;
   private String commandName;
 
   public RetryingExecutor(int maxRetries, long sleepInterval, Command command) {
@@ -47,13 +44,6 @@ public RetryingExecutor(int maxRetries, long sleepInterval, Command command) {
     this.command = command;
   }
 
-  public RetryingExecutor(Configuration conf, Command command) {
-    this.sleepInterval = MetastoreConf.getTimeVar(conf,
-        MetastoreConf.ConfVars.HMS_HANDLER_INTERVAL, TimeUnit.MILLISECONDS);
-    this.maxRetries = MetastoreConf.getIntVar(conf, MetastoreConf.ConfVars.HMS_HANDLER_ATTEMPTS);
-    this.command = command;
-  }
-
   public RetryingExecutor onRetry(Class<? extends Exception> ex) {
     this.retriableException.add(ex);
     return this;

standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java

@@ -1300,7 +1300,11 @@ public void testQueryCloseOnError() throws Exception {
 
   @Test
   public void testRetryingExecutorSleep() throws Exception {
-    RetryingExecutor re = new RetryingExecutor(MetastoreConf.newMetastoreConf(), null);
+    int maxRetries =
+        MetastoreConf.getIntVar(conf, ConfVars.NOTIFICATION_SEQUENCE_LOCK_MAX_RETRIES);
+    long sleepInterval = MetastoreConf.getTimeVar(conf,
+        ConfVars.NOTIFICATION_SEQUENCE_LOCK_RETRY_SLEEP_INTERVAL, TimeUnit.MILLISECONDS);
+    RetryingExecutor re = new RetryingExecutor(maxRetries, sleepInterval, null);
     Assert.assertTrue("invalid sleep value", re.getSleepInterval() >= 0);
   }