[hive] branch master updated: HIVE-23391: Non blocking ALTER TABLE add column (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)

[dk...@apache.org]

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 b0831623e1 HIVE-23391: Non blocking ALTER TABLE add column (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)
b0831623e1 is described below

commit b0831623e1d57518f83a0c95b177d96241ae6b37
Author: Denys Kuzmenko <dk...@cloudera.com>
AuthorDate: Thu May 5 15:22:44 2022 +0200

    HIVE-23391: Non blocking ALTER TABLE add column (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)
    
    Closes #3083
---
 .../apache/hadoop/hive/ql/hooks/WriteEntity.java   |  4 +-
 .../hadoop/hive/ql/lockmgr/TestDbTxnManager2.java  | 82 +++++++++++++++++++++-
 2 files changed, 83 insertions(+), 3 deletions(-)

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 28238ec6b6..8d9079eeb0 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
@@ -223,12 +223,12 @@ public class WriteEntity extends Entity implements Serializable {
     case SET_SKEWED_LOCATION:
     case INTO_BUCKETS:
     case ALTERPARTITION:
-    case ADDCOLS:
     case TRUNCATE:
     case MERGEFILES:
     case OWNER:
       return WriteType.DDL_EXCLUSIVE;
-      
+
+    case ADDCOLS:  
     case ADD_CONSTRAINT: 
     case DROP_CONSTRAINT:
     case RENAME:
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 33c303b2e4..58d0edb107 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
@@ -3514,7 +3514,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{
     FileStatus[] stat = fs.listStatus(new Path(getWarehouseDir()),
       t -> t.getName().matches("tab_acid" + (blocking ? "" : SOFT_DELETE_TABLE_PATTERN)));
     if ((blocking ? 0 : 1) != stat.length) {
-      Assert.fail("Table data was " + (blocking ? "not" : "") + "removed from FS");
+      Assert.fail("Table data was " + (blocking ? "not " : "") + "removed from FS");
     }
     driver.getFetchTask().fetch(res);
     Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, res.size());
@@ -4060,4 +4060,84 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{
     driver.getFetchTask().fetch(res);
     Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, res.size());
   }
+
+  @Test
+  public void testAddColumnsNonBlocking() throws Exception {
+    testAddColumns(false);
+  }
+  @Test
+  public void testAddColumnsBlocking() throws Exception {
+    testAddColumns(true);
+  }
+
+  private void testAddColumns(boolean blocking) throws Exception {
+    dropTable(new String[] {"tab_acid"});
+
+    HiveConf.setBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+    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) " +
+      "stored as orc TBLPROPERTIES ('transactional'='true')");
+    driver.run("insert into tab_acid (a,b) values(1,2),(3,4)");
+
+    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 add columns (c int)");
+
+    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);
+    
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("[1\t2, 3\t4]", res.toString());
+    driver.run("insert into tab_acid values(5,6,7)");
+    
+    driver.run("select * from tab_acid");
+    res = new ArrayList<>();
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("[1\t2\tNULL, 3\t4\tNULL, 5\t6\t7]", res.toString());
+  }
 }