You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/08/13 18:37:17 UTC

[impala] 01/02: IMPALA-8823: DROP TABLE support for insert-only ACID tables

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

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

commit e1b93f27f3fb2a0f3d1c754f8e0e988bad838f11
Author: Gabor Kaszab <ga...@cloudera.com>
AuthorDate: Wed Aug 7 15:02:30 2019 +0200

    IMPALA-8823: DROP TABLE support for insert-only ACID tables
    
    Enhances Impala to be able to drop insert-only transactional tables.
    In order to do this Impala acquires an exclusive table lock in HMS
    before performing the drop operation and releases the lock once
    dropping the table finished.
    INSERT statement does the locking and heartbeating on coordinator
    side but for DROP TABLE all of these are done from Catalog side. This
    means that alongside Impala coordinators now Catalog also does
    heartbeating towards HMS.
    
    Testing:
     - E2E test: Dropped a table, re-created it and dropped again to
       check if no locks remained in HMS.
     - E2E test: After dropping a table from Impala checked if Hive also
       sees it being dropped.
     - Manual test: With a hacked Impala that runs a drop table long
       enough I checked that there is a table lock entry in HMS during the
       execution and disappears once the query finishes.
    
    Change-Id: Ic41ca73268c4b75af5a08fe3dd1ada1df3f6fd34
    Reviewed-on: http://gerrit.cloudera.org:8080/14038
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../org/apache/impala/compat/MetastoreShim.java    | 10 +++-
 .../org/apache/impala/compat/MetastoreShim.java    | 44 +++++++++++++++---
 .../impala/analysis/DropTableOrViewStmt.java       |  2 +-
 .../java/org/apache/impala/catalog/Catalog.java    | 54 ++++++++++++++++++++++
 .../{service => common}/TransactionKeepalive.java  | 39 ++++++++++------
 .../apache/impala/service/CatalogOpExecutor.java   | 25 ++++++++++
 .../java/org/apache/impala/service/Frontend.java   | 14 ++----
 .../org/apache/impala/analysis/AnalyzerTest.java   |  3 +-
 .../functional-query/queries/QueryTest/acid.test   | 34 ++++++++++++--
 tests/metadata/test_hms_integration.py             | 18 ++++++++
 tests/query_test/test_acid.py                      |  2 +-
 11 files changed, 205 insertions(+), 40 deletions(-)

diff --git a/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java b/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
index 93437e9..96bd997 100644
--- a/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
+++ b/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
@@ -329,6 +329,14 @@ public class MetastoreShim {
   /**
    * Hive-3 only function
    */
+  public static void releaseLock(IMetaStoreClient client, long lockId)
+      throws TransactionException {
+    throw new UnsupportedOperationException("releaseLock is not supported.");
+  }
+
+  /**
+   * Hive-3 only function
+   */
   public static boolean heartbeat(IMetaStoreClient client,
       long txnId, long lockId) throws TransactionException {
     throw new UnsupportedOperationException("heartbeat is not supported.");
@@ -338,7 +346,7 @@ public class MetastoreShim {
    * Hive-3 only function
    */
   public static long acquireLock(IMetaStoreClient client, long txnId,
-      List<LockComponent> lockComponents, int lockRetries, int retryWaitSeconds)
+      List<LockComponent> lockComponents)
       throws TransactionException {
     throw new UnsupportedOperationException("acquireLock is not supported.");
   }
diff --git a/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java b/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
index 954327a..18cd0a7 100644
--- a/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
+++ b/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
@@ -72,11 +72,11 @@ import org.apache.impala.authorization.User;
 import org.apache.impala.common.ImpalaException;
 import org.apache.impala.common.Pair;
 import org.apache.impala.common.TransactionException;
+import org.apache.impala.common.TransactionKeepalive;
 import org.apache.impala.compat.HiveMetadataFormatUtils;
 import org.apache.impala.service.BackendConfig;
 import org.apache.impala.service.Frontend;
 import org.apache.impala.service.MetadataOp;
-import org.apache.impala.service.TransactionKeepalive;
 import org.apache.impala.thrift.TMetadataOpRequest;
 import org.apache.impala.thrift.TResultSet;
 import org.apache.log4j.Logger;
@@ -104,6 +104,13 @@ public class MetastoreShim {
   private static final String HIVESQL = "HIVESQL";
   private static final long MAJOR_VERSION = 3;
   private static boolean capabilitiestSet_ = false;
+
+  // Number of retries to acquire an HMS ACID lock.
+  private static final int LOCK_RETRIES = 10;
+
+  // Time interval between retries of acquiring an HMS ACID lock
+  private static final int LOCK_RETRY_WAIT_SECONDS = 3;
+
   /**
    * Wrapper around MetaStoreUtils.validateName() to deal with added arguments.
    */
@@ -539,18 +546,18 @@ public class MetastoreShim {
    * Creates a lock for the given lock components. Returns the acquired lock, this
    * might involve some waiting.
    * @param client is the HMS client to be used.
+   * @param txnId The transaction ID associated with the lock. Zero if the lock doesn't
+   * belong to a transaction.
    * @param lockComponents the lock components to include in this lock.
-   * @param lockRetries the number of retries to acquire the lock.
-   * @param retryWaitSeconds wait interval between retries.
    * @return the lock id
    * @throws TransactionException in case of failure
    */
   public static long acquireLock(IMetaStoreClient client, long txnId,
-      List<LockComponent> lockComponents, int lockRetries, int retryWaitSeconds)
+      List<LockComponent> lockComponents)
           throws TransactionException {
     LockRequestBuilder lockRequestBuilder = new LockRequestBuilder();
     lockRequestBuilder.setUser("Impala");
-    lockRequestBuilder.setTransactionId(txnId);
+    if (txnId > 0) lockRequestBuilder.setTransactionId(txnId);
     for (LockComponent lockComponent : lockComponents) {
       lockRequestBuilder.addLockComponent(lockComponent);
     }
@@ -559,9 +566,9 @@ public class MetastoreShim {
       LockResponse lockResponse = client.lock(lockRequest);
       long lockId = lockResponse.getLockid();
       int retries = 0;
-      while (lockResponse.getState() == LockState.WAITING && retries < lockRetries) {
+      while (lockResponse.getState() == LockState.WAITING && retries < LOCK_RETRIES) {
         try {
-          Thread.sleep(retryWaitSeconds * 1000);
+          Thread.sleep(LOCK_RETRY_WAIT_SECONDS * 1000);
           ++retries;
           lockResponse = client.checkLock(lockId);
         } catch (InterruptedException e) {
@@ -571,6 +578,14 @@ public class MetastoreShim {
         }
       }
       if (lockResponse.getState() == LockState.ACQUIRED) return lockId;
+      if (lockId > 0) {
+        try {
+          releaseLock(client, lockId);
+        } catch (TransactionException te) {
+          LOG.error("Failed to release lock as a cleanup step after acquiring a lock " +
+              "has failed: " + lockId + " " + te.getMessage());
+        }
+      }
       throw new TransactionException("Failed to acquire lock for transaction " +
           String.valueOf(txnId));
     } catch (TException e) {
@@ -579,6 +594,21 @@ public class MetastoreShim {
   }
 
   /**
+   * Releases a lock in HMS.
+   * @param client is the HMS client to be used.
+   * @param lockId is the lock ID to be released.
+   * @throws TransactionException
+   */
+  public static void releaseLock(IMetaStoreClient client, long lockId)
+      throws TransactionException {
+    try {
+      client.unlock(lockId);
+    } catch (Exception e) {
+      throw new TransactionException(e.getMessage());
+    }
+  }
+
+  /**
    * Allocates a write id for the given table.
    * @param client is the HMS client to be used.
    * @param txnId is the transaction id.
diff --git a/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
index 047e285..86da283 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
@@ -121,7 +121,7 @@ public class DropTableOrViewStmt extends StatementBase {
       if (dropTable_) {
         // To drop a view needs not write capabilities, only checks for tables.
         analyzer.checkTableCapability(table, Analyzer.OperationType.WRITE);
-        analyzer.ensureTableNotTransactional(table);
+        analyzer.ensureTableNotFullAcid(table);
       }
 
     } catch (TableLoadingException e) {
diff --git a/fe/src/main/java/org/apache/impala/catalog/Catalog.java b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
index 67441dc..e73de73 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Catalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
@@ -18,6 +18,7 @@
 package org.apache.impala.catalog;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -25,9 +26,18 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.hadoop.hive.metastore.api.DataOperationType;
+import org.apache.hadoop.hive.metastore.api.LockComponent;
+import org.apache.hadoop.hive.metastore.api.LockLevel;
+import org.apache.hadoop.hive.metastore.api.LockType;
+
 import org.apache.impala.analysis.FunctionName;
 import org.apache.impala.authorization.AuthorizationPolicy;
 import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
+import org.apache.impala.common.TransactionException;
+import org.apache.impala.common.TransactionKeepalive;
+import org.apache.impala.common.TransactionKeepalive.HeartbeatContext;
+import org.apache.impala.compat.MetastoreShim;
 import org.apache.impala.thrift.TCatalogObject;
 import org.apache.impala.thrift.TFunction;
 import org.apache.impala.thrift.TPartitionKeyValue;
@@ -88,12 +98,20 @@ public abstract class Catalog implements AutoCloseable {
   protected final CatalogObjectCache<AuthzCacheInvalidation> authzCacheInvalidation_ =
       new CatalogObjectCache<>();
 
+  // This member is responsible for heartbeating HMS locks and transactions.
+  private TransactionKeepalive transactionKeepalive_;
+
   /**
    * Creates a new instance of Catalog backed by a given MetaStoreClientPool.
    */
   public Catalog(MetaStoreClientPool metaStoreClientPool) {
     dataSources_ = new CatalogObjectCache<DataSource>();
     metaStoreClientPool_ = Preconditions.checkNotNull(metaStoreClientPool);
+    if (MetastoreShim.getMajorVersion() > 2) {
+      transactionKeepalive_ = new TransactionKeepalive(metaStoreClientPool_);
+    } else {
+      transactionKeepalive_ = null;
+    }
   }
 
   /**
@@ -647,4 +665,40 @@ public abstract class Catalog implements AutoCloseable {
   public static boolean keyEquals(TCatalogObject first, TCatalogObject second) {
     return toCatalogObjectKey(first).equals(toCatalogObjectKey(second));
   }
+
+  /**
+   * Creates an exclusive lock for a particular table and acquires it in the HMS. Starts
+   * heartbeating the lock. This function is for locks that doesn't belong to a
+   * transaction.
+   * @param dbName Name of the DB where the particular table is.
+   * @param tableName Name of the table where the lock is acquired.
+   * @throws TransactionException
+   */
+  public long lockTable(String dbName, String tableName, HeartbeatContext ctx)
+      throws TransactionException {
+    LockComponent lockComponent = new LockComponent();
+    lockComponent.setDbname(dbName);
+    lockComponent.setTablename(tableName);
+    lockComponent.setLevel(LockLevel.TABLE);
+    lockComponent.setType(LockType.EXCLUSIVE);
+    lockComponent.setOperationType(DataOperationType.NO_TXN);
+    List<LockComponent> lockComponents = Arrays.asList(lockComponent);
+    long lockId = -1L;
+    try (MetaStoreClient client = metaStoreClientPool_.getClient()) {
+      lockId = MetastoreShim.acquireLock(client.getHiveClient(), 0L, lockComponents);
+      transactionKeepalive_.addLock(lockId, ctx);
+    }
+    return lockId;
+  }
+
+  /**
+   * Releases a lock based on its ID from HMS and stops heartbeating it.
+   * @param lockId is the ID of the lock to clear.
+   */
+  public void releaseTableLock(long lockId) throws TransactionException {
+    try (MetaStoreClient client = metaStoreClientPool_.getClient()) {
+      transactionKeepalive_.deleteLock(lockId);
+      MetastoreShim.releaseLock(client.getHiveClient(), lockId);
+    }
+  }
 }
diff --git a/fe/src/main/java/org/apache/impala/service/TransactionKeepalive.java b/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java
similarity index 89%
rename from fe/src/main/java/org/apache/impala/service/TransactionKeepalive.java
rename to fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java
index 21ccbce..f7166f8 100644
--- a/fe/src/main/java/org/apache/impala/service/TransactionKeepalive.java
+++ b/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java
@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-package org.apache.impala.service;
+package org.apache.impala.common;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -54,14 +54,29 @@ public class TransactionKeepalive {
 
   private final MetaStoreClientPool metaStoreClientPool_;
 
+  // Stores information for logging purposes. Stores either a TQueryCtx or a cause
+  // string. toString() returns the stored TQueryCtx if it is set or the string cause
+  // otherwise.
   public static class HeartbeatContext {
-    public TQueryCtx queryCtx;
-    public long creationTime;
+    private TQueryCtx queryCtx;
+    private String cause;
+    private long creationTime;
 
     public HeartbeatContext(TQueryCtx queryCtx, long creationTime) {
       this.queryCtx = queryCtx;
       this.creationTime = creationTime;
     }
+
+    public HeartbeatContext(String cause, long creationTime) {
+      this.queryCtx = null;
+      this.cause = "'" + cause + "'";
+      this.creationTime = creationTime;
+    }
+
+    public String toString() {
+      if (queryCtx != null) return queryCtx.query_id.toString();
+      return cause;
+    }
   }
 
   // Map of transactions
@@ -170,21 +185,19 @@ public class TransactionKeepalive {
           // Transaction or lock doesn't exist anymore, let's remove them.
           if (transactionId != 0) {
             LOG.warn("Transaction " + String.valueOf(transactionId) + " of query " +
-                context.queryCtx.query_id.toString() + " doesn't exist anymore. Stop " +
-                "heartbeating it.");
+                context.toString() + " doesn't exist anymore. Stop heartbeating it.");
             TransactionKeepalive.this.deleteTransaction(transactionId);
           }
           if (lockId != 0) {
             LOG.warn("Lock " + String.valueOf(lockId) + " of query " +
-                context.queryCtx.query_id.toString() + " doesn't exist anymore. Stop " +
-                "heartbeating it.");
+                context.toString() + " doesn't exist anymore. Stop heartbeating it.");
             TransactionKeepalive.this.deleteLock(lockId);
           }
         }
       } catch (TransactionException e) {
         LOG.warn("Caught exception during heartbeating transaction " +
             String.valueOf(transactionId) + " lock " + String.valueOf(lockId) +
-            " for query " + context.queryCtx.query_id.toString(), e);
+            " for query " + context.toString(), e);
       }
     }
   }
@@ -204,22 +217,20 @@ public class TransactionKeepalive {
   /**
    * Add transaction to heartbeat. Associated locks shouldn't be added.
    */
-  synchronized public void addTransaction(Long transactionId, TQueryCtx queryCtx) {
+  synchronized public void addTransaction(Long transactionId, HeartbeatContext ctx) {
     Preconditions.checkNotNull(transactionId);
-    Preconditions.checkNotNull(queryCtx);
+    Preconditions.checkNotNull(ctx);
     Preconditions.checkState(!transactions_.containsKey(transactionId));
-    HeartbeatContext ctx = new HeartbeatContext(queryCtx, System.nanoTime());
     transactions_.put(transactionId, ctx);
   }
 
   /**
    * Add lock to heartbeat. This should be a lock without a transaction context.
    */
-  synchronized public void addLock(Long lockId, TQueryCtx queryCtx) {
+  synchronized public void addLock(Long lockId, HeartbeatContext ctx) {
     Preconditions.checkNotNull(lockId);
-    Preconditions.checkNotNull(queryCtx);
+    Preconditions.checkNotNull(ctx);
     Preconditions.checkState(!locks_.containsKey(lockId));
-    HeartbeatContext ctx = new HeartbeatContext(queryCtx, System.nanoTime());
     locks_.put(lockId, ctx);
   }
 
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 7ad8b36..643a551 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -100,6 +100,7 @@ import org.apache.impala.common.JniUtil;
 import org.apache.impala.common.Pair;
 import org.apache.impala.common.Reference;
 import org.apache.impala.common.TransactionException;
+import org.apache.impala.common.TransactionKeepalive.HeartbeatContext;
 import org.apache.impala.compat.MetastoreShim;
 import org.apache.impala.thrift.JniCatalogConstants;
 import org.apache.impala.thrift.TAlterDbParams;
@@ -1546,6 +1547,8 @@ public class CatalogOpExecutor {
    * Also drops all associated caching requests on the table and/or table's partitions,
    * uncaching all table data. If params.purge is true, table data is permanently
    * deleted.
+   * In case of transactional tables acquires an exclusive HMS table lock before
+   * executing the drop operation.
    */
   private void dropTableOrView(TDropTableOrViewParams params, TDdlExecResponse resp)
       throws ImpalaException {
@@ -1571,6 +1574,28 @@ public class CatalogOpExecutor {
       // or non-existence of the database will be handled down below.
     }
 
+    Table tbl = catalog_.getTableIfCachedNoThrow(tableName.getDb(), tableName.getTbl());
+    long lockId = -1;
+    if (tbl != null && !(tbl instanceof IncompleteTable) &&
+        AcidUtils.isTransactionalTable(tbl.getMetaStoreTable().getParameters())) {
+      HeartbeatContext ctx = new HeartbeatContext(
+          String.format("Drop table/view %s.%s", tableName.getDb(), tableName.getTbl()),
+          System.nanoTime());
+      lockId = catalog_.lockTable(tableName.getDb(), tableName.getTbl(), ctx);
+    }
+
+    try {
+      dropTableOrViewInternal(params, tableName, resp);
+    } finally {
+      if (lockId > 0) catalog_.releaseTableLock(lockId);
+    }
+  }
+
+  /**
+   * Helper function for dropTableOrView().
+   */
+  private void dropTableOrViewInternal(TDropTableOrViewParams params,
+      TableName tableName, TDdlExecResponse resp) throws ImpalaException {
     TCatalogObject removedObject = new TCatalogObject();
     synchronized (metastoreDdlLock_) {
       Db db = catalog_.getDb(params.getTable_name().db_name);
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 72c8764..e09164b 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -103,6 +103,8 @@ import org.apache.impala.common.ImpalaException;
 import org.apache.impala.common.InternalException;
 import org.apache.impala.common.NotImplementedException;
 import org.apache.impala.common.TransactionException;
+import org.apache.impala.common.TransactionKeepalive;
+import org.apache.impala.common.TransactionKeepalive.HeartbeatContext;
 import org.apache.impala.compat.MetastoreShim;
 import org.apache.impala.hooks.QueryCompleteContext;
 import org.apache.impala.hooks.QueryEventHook;
@@ -188,12 +190,6 @@ public class Frontend {
   private static final int INCONSISTENT_METADATA_NUM_RETRIES =
       BackendConfig.INSTANCE.getLocalCatalogMaxFetchRetries();
 
-  // Number of retries to acquire an HMS ACID lock.
-  private static final int LOCK_RETRIES = 10;
-
-  // Time interval between retries of acquiring an HMS ACID lock
-  private static final int LOCK_RETRY_WAIT_SECONDS = 3;
-
   /**
    * Plan-time context that allows capturing various artifacts created
    * during the process.
@@ -1675,7 +1671,8 @@ public class Frontend {
     try (MetaStoreClient client = metaStoreClientPool_.getClient()) {
       IMetaStoreClient hmsClient = client.getHiveClient();
       long transactionId = MetastoreShim.openTransaction(hmsClient, "Impala");
-      transactionKeepalive_.addTransaction(transactionId, queryCtx);
+      HeartbeatContext ctx = new HeartbeatContext(queryCtx, System.nanoTime());
+      transactionKeepalive_.addTransaction(transactionId, ctx);
       return transactionId;
     }
   }
@@ -1763,8 +1760,7 @@ public class Frontend {
     }
     try (MetaStoreClient client = metaStoreClientPool_.getClient()) {
       IMetaStoreClient hmsClient = client.getHiveClient();
-      MetastoreShim.acquireLock(hmsClient, txnId, lockComponents, LOCK_RETRIES,
-          LOCK_RETRY_WAIT_SECONDS);
+      MetastoreShim.acquireLock(hmsClient, txnId, lockComponents);
     }
   }
 }
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
index 57803f1..85a977f 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
@@ -583,8 +583,7 @@ public class AnalyzerTest extends FrontendTestBase {
     AnalysisError(
         "drop table functional_orc_def.full_transactional_table",
         insertOnlyErrorForFullMsg);
-    AnalysisError("drop table functional.insert_only_transactional_table",
-        insertOnlyErrorMsg);
+    AnalyzesOk("drop table functional.insert_only_transactional_table");
 
     AnalysisError(
         "truncate table functional_orc_def.full_transactional_table",
diff --git a/testdata/workloads/functional-query/queries/QueryTest/acid.test b/testdata/workloads/functional-query/queries/QueryTest/acid.test
index dac75f4..2939685 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/acid.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/acid.test
@@ -1,5 +1,6 @@
 ====
 ---- HIVE_QUERY
+# Create a table with Hive and run insert, select, and drop from Impala on it.
 use $DATABASE;
 create table tt (x int) tblproperties (
   'transactional'='true',
@@ -14,6 +15,7 @@ select * from tt
 1
 ====
 ---- HIVE_QUERY
+# Insert from Hive to test refresh table from Impala in the below test.
 use $DATABASE;
 insert into tt values (2);
 ====
@@ -32,8 +34,7 @@ select * from tt order by x;
 1
 2
 ====
----- HIVE_QUERY
-use $DATABASE;
+---- QUERY
 insert overwrite table tt values (3);
 insert into tt values (4);
 ====
@@ -44,8 +45,7 @@ select * from tt order by x;
 3
 4
 ====
----- HIVE_QUERY
-use $DATABASE;
+---- QUERY
 create table upgraded_table (x int);
 insert into upgraded_table values (1);
 # Upgrade to the table to insert only acid when there are already values in it.
@@ -55,10 +55,34 @@ insert into upgraded_table values (2);
 insert into upgraded_table values (3);
 ====
 ---- QUERY
-invalidate metadata upgraded_table;
 select * from upgraded_table;
 ---- RESULTS
 1
 2
 3
 ====
+---- QUERY
+drop table tt;
+show tables;
+---- RESULTS
+'upgraded_table'
+====
+---- QUERY
+# After dropping the table I re-create and drop it again to check that all the locks
+# are released properly from HMS.
+create table tt (x int) tblproperties (
+  'transactional'='true',
+  'transactional_properties'='insert_only');
+====
+---- QUERY
+show tables;
+---- RESULTS
+'upgraded_table'
+'tt'
+====
+---- QUERY
+drop table tt;
+show tables;
+---- RESULTS
+'upgraded_table'
+====
diff --git a/tests/metadata/test_hms_integration.py b/tests/metadata/test_hms_integration.py
index 9b1fd04..8df23cb 100644
--- a/tests/metadata/test_hms_integration.py
+++ b/tests/metadata/test_hms_integration.py
@@ -726,6 +726,24 @@ class TestHmsIntegration(ImpalaTestSuite):
     assert '3,30' == hive_result[3]
     assert '4,41' == hive_result[4]
 
+  @SkipIfHive2.acid
+  def test_drop_acid_table(self, vector, unique_database):
+    """
+    Tests that a transactional table dropped by Impala is also dropped if we check from
+    Hive.
+    """
+    table_name = "%s.acid_insert" % unique_database
+    self.client.execute(
+      "create table %s (i int) "
+      "TBLPROPERTIES('transactional'='true', "
+      "'transactional_properties'='insert_only')" % table_name)
+    show_tables_result = self.run_stmt_in_hive("show tables in %s" % unique_database)
+    assert "acid_insert" in show_tables_result
+    self.client.execute("drop table %s" % table_name)
+    show_tables_result_after_drop = self.run_stmt_in_hive(
+        "show tables in %s" % unique_database)
+    assert "acid_insert" not in show_tables_result_after_drop
+
   @pytest.mark.execute_serially
   def test_change_table_name(self, vector):
     """
diff --git a/tests/query_test/test_acid.py b/tests/query_test/test_acid.py
index 6ac72dd..8b3ef75 100644
--- a/tests/query_test/test_acid.py
+++ b/tests/query_test/test_acid.py
@@ -46,7 +46,7 @@ class TestAcid(ImpalaTestSuite):
   @SkipIfADLS.hive
   @SkipIfIsilon.hive
   @SkipIfLocal.hive
-  def test_acid(self, vector, unique_database):
+  def test_acid_basic(self, vector, unique_database):
     self.run_test_case('QueryTest/acid', vector, use_db=unique_database)
 
   @SkipIfHive2.acid