You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by dk...@apache.org on 2022/05/04 07:51:44 UTC

[hive] branch master updated: HIVE-26076: Non blocking ADD PARTITION if not exists (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)

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 71c04dd5bb HIVE-26076: Non blocking ADD PARTITION if not exists (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)
71c04dd5bb is described below

commit 71c04dd5bb51176eb79052818aa179b0b535c6d8
Author: Denys Kuzmenko <dk...@cloudera.com>
AuthorDate: Wed May 4 09:51:33 2022 +0200

    HIVE-26076: Non blocking ADD PARTITION if not exists (Denys Kuzmenko, reviewed by Antal Sinkovits, Laszlo Vegh)
    
    Closes #3122
---
 .../add/AbstractAddPartitionAnalyzer.java          |  6 +-
 .../apache/hadoop/hive/ql/TestTxnAddPartition.java | 13 +---
 .../hadoop/hive/ql/lockmgr/TestDbTxnManager2.java  | 78 +++++++++++++++-------
 3 files changed, 59 insertions(+), 38 deletions(-)

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AbstractAddPartitionAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AbstractAddPartitionAnalyzer.java
index 9da2daa8ec..b8433d2617 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AbstractAddPartitionAnalyzer.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AbstractAddPartitionAnalyzer.java
@@ -59,11 +59,11 @@ abstract class AbstractAddPartitionAnalyzer extends AbstractAlterTableAnalyzer {
 
     boolean ifNotExists = command.getChild(0).getType() == HiveParser.TOK_IFNOTEXISTS;
     outputs.add(new WriteEntity(table,
-        /* use DDL_EXCLUSIVE to cause X lock to prevent races between concurrent add partition calls with IF NOT EXISTS.
+        /* use DDL_EXCL_WRITE to cause X_WRITE lock to prevent races between concurrent add partition calls with IF NOT EXISTS.
          * w/o this 2 concurrent calls to add the same partition may both add data since for transactional tables
          * creating partition metadata and moving data there are 2 separate actions. */
-        ifNotExists && AcidUtils.isTransactionalTable(table) ?
-            WriteType.DDL_EXCLUSIVE : WriteEntity.WriteType.DDL_SHARED));
+      ifNotExists && AcidUtils.isTransactionalTable(table) ?
+          WriteType.DDL_EXCL_WRITE : WriteType.DDL_SHARED));
 
     List<AlterTableAddPartitionDesc.PartitionDesc> partitions = createPartitions(command, table, ifNotExists);
     if (partitions.isEmpty()) { // nothing to do
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnAddPartition.java b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnAddPartition.java
index fa15e2876a..d07a2281a1 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnAddPartition.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnAddPartition.java
@@ -21,10 +21,8 @@ package org.apache.hadoop.hive.ql;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.conf.HiveConf;
-import org.apache.hadoop.hive.ql.lockmgr.TestDbTxnManager2;
 import org.apache.hadoop.hive.ql.processors.CommandProcessorException;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -269,16 +267,7 @@ public class TestTxnAddPartition extends TxnCommandsBaseForTests {
             "warehouse/t/p=0/delta_0000001_0000001_0000/000001_0"}};
     checkExpected(rs, expected, "add partition (p=0)");
   }
-
-  /**
-   * {@link TestDbTxnManager2#testAddPartitionLocks}
-   */
-  @Ignore
-  @Test
-  public void testLocks() throws Exception {
-  }
-
-
+  
   @Test
   public void addPartitionTransactional() throws Exception {
     exception.expect(RuntimeException.class);
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 613e3ab858..33c303b2e4 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
@@ -3116,29 +3116,6 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{
   @Rule
   public TemporaryFolder exportFolder = new TemporaryFolder();
 
-  /**
-   * see also {@link org.apache.hadoop.hive.ql.TestTxnAddPartition}
-   */
-  @Test
-  public void testAddPartitionLocks() throws Exception {
-    dropTable(new String[] {"T", "Tstage"});
-    driver.run("create table T (a int, b int) partitioned by (p int) " +
-        "stored as orc tblproperties('transactional'='true')");
-    //bucketed just so that we get 2 files
-    driver.run("create table Tstage (a int, b int)  clustered by (a) into 2 " +
-        "buckets stored as orc tblproperties('transactional'='false')");
-    driver.run("insert into Tstage values(0,2),(1,4)");
-    String exportLoc = exportFolder.newFolder("1").toString();
-    driver.run("export table Tstage to '" + exportLoc + "'");
-
-    driver.compileAndRespond("ALTER TABLE T ADD if not exists PARTITION (p=0) location '" + exportLoc + "/data'", true);
-    txnMgr.acquireLocks(driver.getPlan(), ctx, "Fifer"); //gets X lock on T
-
-    List<ShowLocksResponseElement> locks = getLocks();
-    Assert.assertEquals("Unexpected lock count", 1, locks.size());
-    checkLock(LockType.EXCLUSIVE, LockState.ACQUIRED, "default", "T", null, locks);
-  }
-
   @Test
   public void testLoadData() throws Exception {
     testLoadData(false);
@@ -4028,4 +4005,59 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{
     checkLock(LockType.EXCL_WRITE,
       LockState.ACQUIRED, "default", "tab_acid", null, locks);
   }
+
+  @Test
+  public void testAddPartitionIfNotExists() throws Exception {
+    dropTable(new String[] {"T", "Tstage"});
+    
+    HiveConf.setIntVar(conf, HiveConf.ConfVars.HIVE_LOCKS_PARTITION_THRESHOLD, 1);
+    driver = Mockito.spy(driver);
+    driver2 = Mockito.spy(driver2);
+    
+    driver.run("create table if not exists T (a int, b int) partitioned by (p string) " +
+      "stored as orc TBLPROPERTIES ('transactional'='true')");
+    //bucketed just so that we get 2 files
+    driver.run("create table if not exists Tstage (a int, b int) clustered by (a) into 2 " +
+      "buckets stored as orc TBLPROPERTIES ('transactional'='false')");
+    driver.run("insert into Tstage values(1,2),(3,4)");
+    String exportLoc = exportFolder.newFolder("1").toString();
+    driver.run("export table Tstage to '" + exportLoc + "'");
+    
+    driver.compileAndRespond("select * from T");
+    
+    driver.lockAndRespond();
+    List<ShowLocksResponseElement> locks = getLocks();
+    Assert.assertEquals("Unexpected lock count", 1, locks.size());
+
+    checkLock(LockType.SHARED_READ,
+      LockState.ACQUIRED, "default", "T", null, locks);
+
+    DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+    swapTxnManager(txnMgr2);
+    driver2.compileAndRespond("alter table T add if not exists partition (p='foo') location '" + exportLoc + "/data'");
+    
+    driver2.lockAndRespond();
+    locks = getLocks();
+    Assert.assertEquals("Unexpected lock count", 2, locks.size());
+
+    checkLock(LockType.EXCL_WRITE,
+      LockState.ACQUIRED, "default", "T", null, locks);
+
+    Mockito.doNothing().when(driver2).lockAndRespond();
+    driver2.run();
+    
+    swapTxnManager(txnMgr);
+    Mockito.doNothing().when(driver).lockAndRespond();
+    driver.run();
+    Mockito.reset(driver, driver2);
+
+    List<String> res = new ArrayList<>();
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("Expecting 0 rows and found " + res.size(), 0, res.size());
+    
+    driver.run("select * from T where p='foo'");
+    res = new ArrayList<>();
+    driver.getFetchTask().fetch(res);
+    Assert.assertEquals("Expecting 2 rows and found " + res.size(), 2, res.size());
+  }
 }