Repository: hive
Updated Branches:
  refs/heads/master a263f0831 -> b290468c0


HIVE-18783: ALTER TABLE post-commit listener does not include the transactional 
listener responses (Sergio Pena, reviewed by Vihang Karajgaonkar)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/b290468c
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/b290468c
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/b290468c

Branch: refs/heads/master
Commit: b290468c0ffe53daa76fad1e2a063d4596ea2ece
Parents: a263f08
Author: Sergio Pena <sergio.p...@cloudera.com>
Authored: Mon Apr 9 10:04:43 2018 -0500
Committer: Sergio Pena <sergio.p...@cloudera.com>
Committed: Mon Apr 9 10:04:43 2018 -0500

----------------------------------------------------------------------
 .../listener/TestDbNotificationListener.java    |  1 +
 .../hadoop/hive/metastore/HiveAlterHandler.java | 59 ++++++++++++++++++--
 .../hadoop/hive/metastore/HiveMetaStore.java    | 30 ++--------
 .../hadoop/hive/metastore/IHMSHandler.java      |  6 ++
 4 files changed, 65 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/b290468c/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
----------------------------------------------------------------------
diff --git 
a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 
b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
index 823312b..70c6a94 100644
--- 
a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
+++ 
b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
@@ -472,6 +472,7 @@ public class TestDbNotificationListener {
     assertEquals(TableType.MANAGED_TABLE.toString(), 
alterTableMessage.getTableType());
 
     // Verify the eventID was passed to the non-transactional listener
+    MockMetaStoreEventListener.popAndVerifyLastEventId(EventType.ALTER_TABLE, 
firstEventId + 2);
     MockMetaStoreEventListener.popAndVerifyLastEventId(EventType.CREATE_TABLE, 
firstEventId + 1);
 
     // When hive.metastore.transactional.event.listeners is set,

http://git-wip-us.apache.org/repos/asf/hive/blob/b290468c/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index ed1b8c5..60bed98 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -55,6 +55,7 @@ import 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
 import java.io.IOException;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -122,15 +123,24 @@ public class HiveAlterHandler implements AlterHandler {
 
     boolean success = false;
     boolean dataWasMoved = false;
-    Table oldt;
+    boolean isPartitionedTable = false;
+
+    Table oldt = null;
+
     List<TransactionalMetaStoreEventListener> transactionalListeners = null;
+    List<MetaStoreEventListener> listeners = null;
+    Map<String, String> txnAlterTableEventResponses = Collections.emptyMap();
+    Map<String, String> txnDropTableEventResponses = Collections.emptyMap();
+    Map<String, String> txnCreateTableEventResponses = Collections.emptyMap();
+    Map<String, String> txnAddPartitionEventResponses = Collections.emptyMap();
+
     if (handler != null) {
       transactionalListeners = handler.getTransactionalListeners();
+      listeners = handler.getListeners();
     }
 
     try {
       boolean rename = false;
-      boolean isPartitionedTable = false;
       List<Partition> parts;
 
       // Switching tables between catalogs is not allowed.
@@ -337,23 +347,23 @@ public class HiveAlterHandler implements AlterHandler {
 
       if (transactionalListeners != null && !transactionalListeners.isEmpty()) 
{
         if (oldt.getDbName().equalsIgnoreCase(newt.getDbName())) {
-          MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
+          txnAlterTableEventResponses = 
MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
                   EventMessage.EventType.ALTER_TABLE,
                   new AlterTableEvent(oldt, newt, false, true, handler),
                   environmentContext);
         } else {
-          MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
+          txnDropTableEventResponses = 
MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
                   EventMessage.EventType.DROP_TABLE,
                   new DropTableEvent(oldt, true, false, handler),
                   environmentContext);
-          MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
+          txnCreateTableEventResponses = 
MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
                   EventMessage.EventType.CREATE_TABLE,
                   new CreateTableEvent(newt, true, handler),
                   environmentContext);
           if (isPartitionedTable) {
             String cName = newt.isSetCatName() ? newt.getCatName() : 
DEFAULT_CATALOG_NAME;
             parts = msdb.getPartitions(cName, newt.getDbName(), 
newt.getTableName(), -1);
-            MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
+            txnAddPartitionEventResponses = 
MetaStoreListenerNotifier.notifyEvent(transactionalListeners,
                     EventMessage.EventType.ADD_PARTITION,
                     new AddPartitionEvent(newt, parts, true, handler),
                     environmentContext);
@@ -397,6 +407,43 @@ public class HiveAlterHandler implements AlterHandler {
         }
       }
     }
+
+    if (!listeners.isEmpty()) {
+      // An ALTER_TABLE event will be created for any alter table operation 
happening inside the same
+      // database, otherwise a rename between databases is considered a 
DROP_TABLE from the old database
+      // and a CREATE_TABLE in the new database plus ADD_PARTITION operations 
if needed.
+      if (!success || dbname.equalsIgnoreCase(newDbName)) {
+        // I don't think event notifications in case of failures are 
necessary, but other HMS operations
+        // make this call whether the event failed or succeeded. To make this 
behavior consistent, then
+        // this call will be made also for failed events even for renaming the 
table between databases
+        // to avoid a large list of ADD_PARTITION unnecessary failed events.
+        MetaStoreListenerNotifier.notifyEvent(listeners, 
EventMessage.EventType.ALTER_TABLE,
+            new AlterTableEvent(oldt, newt, false, success, handler),
+            environmentContext, txnAlterTableEventResponses, msdb);
+      } else {
+        MetaStoreListenerNotifier.notifyEvent(listeners, 
EventMessage.EventType.DROP_TABLE,
+            new DropTableEvent(oldt, true, false, handler),
+            environmentContext, txnDropTableEventResponses, msdb);
+
+        MetaStoreListenerNotifier.notifyEvent(listeners, 
EventMessage.EventType.CREATE_TABLE,
+            new CreateTableEvent(newt, true, handler),
+            environmentContext, txnCreateTableEventResponses, msdb);
+
+        if (isPartitionedTable) {
+          try {
+            List<Partition> parts = msdb.getPartitions(catName, newDbName, 
newTblName, -1);
+            MetaStoreListenerNotifier.notifyEvent(listeners, 
EventMessage.EventType.ADD_PARTITION,
+                new AddPartitionEvent(newt, parts, true, handler),
+                environmentContext, txnAddPartitionEventResponses, msdb);
+          } catch (NoSuchObjectException e) {
+            // Just log the error but not throw an exception as this 
post-commit event should
+            // not cause the HMS operation to fail.
+            LOG.error("ADD_PARTITION events for ALTER_TABLE rename operation 
cannot continue because the following " +
+                "table was not found on the metastore: " + newDbName + "." + 
newTblName, e);
+          }
+        }
+      }
+    }
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hive/blob/b290468c/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 450da4f..102e5b4 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -498,6 +498,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     }
 
     @Override
+    public List<MetaStoreEventListener> getListeners() {
+      return listeners;
+    }
+
+    @Override
     public void init() throws MetaException {
       initListeners = MetaStoreUtils.getMetaStoreListeners(
           MetaStoreInitListener.class, conf, MetastoreConf.getVar(conf, 
ConfVars.INIT_HOOKS));
@@ -4846,31 +4851,6 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         alterHandler.alterTable(getMS(), wh, catName, dbname, name, newTable,
                 envContext, this);
         success = true;
-        if (!listeners.isEmpty()) {
-          if (oldt.getDbName().equalsIgnoreCase(newTable.getDbName())) {
-            MetaStoreListenerNotifier.notifyEvent(listeners,
-                    EventType.ALTER_TABLE,
-                    new AlterTableEvent(oldt, newTable, false, true, this),
-                    envContext);
-          } else {
-            MetaStoreListenerNotifier.notifyEvent(listeners,
-                    EventType.DROP_TABLE,
-                    new DropTableEvent(oldt, true, false, this),
-                    envContext);
-            MetaStoreListenerNotifier.notifyEvent(listeners,
-                    EventType.CREATE_TABLE,
-                    new CreateTableEvent(newTable, true, this),
-                    envContext);
-            if (newTable.getPartitionKeysSize() != 0) {
-              List<Partition> partitions = getMS().getPartitions(catName,
-                  newTable.getDbName(), newTable.getTableName(), -1);
-              MetaStoreListenerNotifier.notifyEvent(listeners,
-                      EventType.ADD_PARTITION,
-                      new AddPartitionEvent(newTable, partitions, true, this),
-                      envContext);
-            }
-          }
-        }
       } catch (NoSuchObjectException e) {
         // thrown when the table to be altered does not exist
         ex = e;

http://git-wip-us.apache.org/repos/asf/hive/blob/b290468c/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
index f59f40b..1a81dc9 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
@@ -110,4 +110,10 @@ public interface IHMSHandler extends 
ThriftHiveMetastore.Iface, Configurable {
    * @return list of listeners.
    */
   List<TransactionalMetaStoreEventListener> getTransactionalListeners();
+
+  /**
+   * Get a list of all non-transactional listeners.
+   * @return list of non-transactional listeners.
+   */
+  List<MetaStoreEventListener> getListeners();
 }

Reply via email to