This is an automated email from the ASF dual-hosted git repository.

dkuzmenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git


The following commit(s) were added to refs/heads/master by this push:
     new ff26c39  HIVE-25956: Non blocking RENAME TABLE implementation (Denys 
Kuzmenko, reviewed by Laszlo Pinter)
ff26c39 is described below

commit ff26c392d58439267af48d80a59a92dabb5cb3ce
Author: Denys Kuzmenko <[email protected]>
AuthorDate: Mon Mar 21 08:50:40 2022 +0100

    HIVE-25956: Non blocking RENAME TABLE implementation (Denys Kuzmenko, 
reviewed by Laszlo Pinter)
    
    Closes #3022
---
 .../ddl/table/AbstractBaseAlterTableAnalyzer.java  |   2 +-
 .../drop/AlterTableDropConstraintAnalyzer.java     |   2 +-
 .../apache/hadoop/hive/ql/hooks/WriteEntity.java   |  10 +-
 .../org/apache/hadoop/hive/ql/io/AcidUtils.java    |   5 +
 .../hadoop/hive/ql/lockmgr/TestDbTxnManager2.java  | 102 +++++++++++++++++++++
 .../hadoop/hive/metastore/HiveAlterHandler.java    |   1 +
 6 files changed, 118 insertions(+), 4 deletions(-)

diff --git 
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractBaseAlterTableAnalyzer.java
 
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractBaseAlterTableAnalyzer.java
index e48926e..03aa022 100644
--- 
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractBaseAlterTableAnalyzer.java
+++ 
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractBaseAlterTableAnalyzer.java
@@ -132,7 +132,7 @@ public abstract class AbstractBaseAlterTableAnalyzer 
extends BaseSemanticAnalyze
       // See HIVE-16688 for use cases.
       return WriteType.DDL_EXCLUSIVE;
     }
-    return WriteEntity.determineAlterTableWriteType(op);
+    return WriteEntity.determineAlterTableWriteType(op, table, conf);
   }
 
   protected void validateAlterTableType(Table table, AlterTableType op, 
boolean expectView)
diff --git 
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constraint/drop/AlterTableDropConstraintAnalyzer.java
 
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constraint/drop/AlterTableDropConstraintAnalyzer.java
index 2dfa4c4..8392d31 100644
--- 
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constraint/drop/AlterTableDropConstraintAnalyzer.java
+++ 
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constraint/drop/AlterTableDropConstraintAnalyzer.java
@@ -58,7 +58,7 @@ public class AlterTableDropConstraintAnalyzer extends 
AbstractAlterTableAnalyzer
       setAcidDdlDesc(desc);
       writeType = WriteType.DDL_EXCLUSIVE;
     } else {
-      writeType = 
WriteEntity.determineAlterTableWriteType(AlterTableType.DROP_CONSTRAINT);
+      writeType = 
WriteEntity.determineAlterTableWriteType(AlterTableType.DROP_CONSTRAINT, table, 
conf);
     }
     inputs.add(new ReadEntity(table));
     WriteEntity alterTableOutput = new WriteEntity(table, writeType);
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 
b/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java
index 677ef30..21ae79a 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java
@@ -18,10 +18,13 @@
 
 package org.apache.hadoop.hive.ql.hooks;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.api.DataConnector;
 import org.apache.hadoop.hive.metastore.api.Database;
 import org.apache.hadoop.hive.ql.ddl.table.AlterTableType;
+import org.apache.hadoop.hive.ql.io.AcidUtils;
 import org.apache.hadoop.hive.ql.metadata.DummyPartition;
 import org.apache.hadoop.hive.ql.metadata.Partition;
 import org.apache.hadoop.hive.ql.metadata.Table;
@@ -202,7 +205,7 @@ public class WriteEntity extends Entity implements 
Serializable {
    * @param op Operation type from the alter table description
    * @return the write type this should use.
    */
-  public static WriteType determineAlterTableWriteType(AlterTableType op) {
+  public static WriteType determineAlterTableWriteType(AlterTableType op, 
Table table, Configuration conf) {
     switch (op) {
     case RENAME_COLUMN:
     case CLUSTERED_BY:
@@ -222,7 +225,6 @@ public class WriteEntity extends Entity implements 
Serializable {
     case INTO_BUCKETS:
     case ALTERPARTITION:
     case ADDCOLS:
-    case RENAME:
     case TRUNCATE:
     case MERGEFILES:
     case ADD_CONSTRAINT:
@@ -230,6 +232,10 @@ public class WriteEntity extends Entity implements 
Serializable {
     case OWNER:
       return WriteType.DDL_EXCLUSIVE;
 
+    case RENAME:
+      return AcidUtils.isLocklessReadsSupported(table, conf) ? 
+          WriteType.DDL_EXCL_WRITE : WriteType.DDL_EXCLUSIVE;
+
     case ADDPARTITION:
     case SET_SERDE_PROPS:
     case ADDPROPS:
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 
b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
index 3dda052..ebf2d79 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
@@ -409,6 +409,11 @@ public class AcidUtils {
         + String.format(DELTA_DIGITS, visibilityTxnId);
   }
 
+  public static boolean isLocklessReadsSupported(Table table, Configuration 
conf) {
+    return HiveConf.getBoolVar(conf, 
HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED)
+        && AcidUtils.isTransactionalTable(table);
+  }
+
   /**
    * Represents bucketId and copy_N suffix
    */
diff --git 
a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
index b78a93f..c3f4cd6 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
@@ -3592,4 +3592,106 @@ public class TestDbTxnManager2 extends 
DbTxnManagerEndToEndTestBase{
     driver.getFetchTask().fetch(res);
     Assert.assertEquals("No records found", 2, res.size());
   }
+  
+  @Test
+  public void testRenameTableNonBlocking() throws Exception {
+    testRenameTable(false);
+  }
+  @Test
+  public void testRenameTableBlocking() throws Exception {
+    testRenameTable(true);
+  }
+
+  private void testRenameTable(boolean blocking) throws Exception {
+    dropTable(new String[] {"tab_acid", "tab_acid_v2"});
+    FileSystem fs = FileSystem.get(conf);
+
+    HiveConf.setBoolVar(conf, 
HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+    HiveConf.setIntVar(conf, HiveConf.ConfVars.HIVE_LOCKS_PARTITION_THRESHOLD, 
1);
+    driver = Mockito.spy(driver);
+
+    HiveConf.setBoolVar(driver2.getConf(), 
HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+    driver2 = Mockito.spy(driver2);
+
+    driver.run("create table if not exists tab_acid (a int, b int) partitioned 
by (p string) " +
+      "stored as orc TBLPROPERTIES ('transactional'='true')");
+    driver.run("insert into tab_acid partition(p) (a,b,p) 
values(1,2,'foo'),(3,4,'bar')");
+
+    driver.compileAndRespond("select * from tab_acid");
+    List<String> res = new ArrayList<>();
+
+    driver.lockAndRespond();
+    List<ShowLocksResponseElement> locks = getLocks();
+    Assert.assertEquals("Unexpected lock count", 1, locks.size());
+
+    checkLock(LockType.SHARED_READ,
+      LockState.ACQUIRED, "default", "tab_acid", null, locks);
+
+    DbTxnManager txnMgr2 = (DbTxnManager) 
TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+    swapTxnManager(txnMgr2);
+    driver2.compileAndRespond("alter table tab_acid rename to tab_acid_v2");
+
+    if (blocking) {
+      txnMgr2.acquireLocks(driver2.getPlan(), ctx, null, false);
+      locks = getLocks();
+
+      ShowLocksResponseElement checkLock = checkLock(LockType.EXCLUSIVE,
+        LockState.WAITING, "default", "tab_acid", null, locks);
+
+      swapTxnManager(txnMgr);
+      Mockito.doNothing().when(driver).lockAndRespond();
+      driver.run();
+
+      driver.getFetchTask().fetch(res);
+      swapTxnManager(txnMgr2);
+
+      FieldSetter.setField(txnMgr2, 
txnMgr2.getClass().getDeclaredField("numStatements"), 0);
+      txnMgr2.getMS().unlock(checkLock.getLockid());
+    }
+    driver2.lockAndRespond();
+    locks = getLocks();
+    Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, 
locks.size());
+
+    checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE,
+      LockState.ACQUIRED, "default", "tab_acid", null, locks);
+
+    Mockito.doNothing().when(driver2).lockAndRespond();
+    driver2.run();
+
+    if (!blocking) {
+      swapTxnManager(txnMgr);
+      Mockito.doNothing().when(driver).lockAndRespond();
+      driver.run();
+    }
+    Mockito.reset(driver, driver2);
+
+    FileStatus[] stat = fs.listStatus(new Path(getWarehouseDir()),
+      t -> t.getName().matches("tab_acid" + (blocking ? "_v2" : 
SOFT_DELETE_TABLE_PATTERN)));
+    if (1 != stat.length) {
+      Assert.fail("Table couldn't be found on FS");
+    }
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, 
res.size());
+
+    try {
+      driver.run("select * from tab_acid");
+    } catch (CommandProcessorException ex) {
+      Assert.assertEquals(ErrorMsg.INVALID_TABLE.getErrorCode(), 
ex.getResponseCode());
+    }
+
+    driver.run("select * from tab_acid_v2");
+    res = new ArrayList<>();
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, 
res.size());
+
+    //create table with the same name
+    driver.run("create table if not exists tab_acid (a int, b int) partitioned 
by (p string) " +
+      "stored as orc TBLPROPERTIES ('transactional'='true')");
+    driver.run("insert into tab_acid partition(p) (a,b,p) 
values(1,2,'foo'),(3,4,'bar')");
+
+    driver.run("select * from tab_acid ");
+    res = new ArrayList<>();
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, 
res.size());
+  }
 }
diff --git 
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index 283ea5b..aa32d60 100644
--- 
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++ 
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -245,6 +245,7 @@ public class HiveAlterHandler implements AlterHandler {
           // in the table rename, its data location should not be changed. We 
can check
           // if the table directory was created directly under its database 
directory to tell
           // if it is such a table
+          // Same applies to the ACID tables suffixed with the `txnId`, case 
with `lockless reads`. 
           String oldtRelativePath = wh.getDatabaseManagedPath(olddb).toUri()
               .relativize(srcPath.toUri()).toString();
           boolean tableInSpecifiedLoc = 
!oldtRelativePath.equalsIgnoreCase(name)

Reply via email to