HIVE-17150: CREATE INDEX execute HMS out-of-transaction listener calls inside a transaction (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/cd39cf38 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/cd39cf38 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/cd39cf38 Branch: refs/heads/branch-2.3 Commit: cd39cf38aae32ac39cb1adf92966be32ab796a6a Parents: aef5ebb Author: Sergio Pena <sergio.p...@cloudera.com> Authored: Mon Jul 24 16:52:49 2017 -0500 Committer: Sahil Takiar <stak...@cloudera.com> Committed: Tue Nov 7 08:15:47 2017 -0800 ---------------------------------------------------------------------- .../listener/DbNotificationListener.java | 1 + .../MetaStoreEventListenerConstants.java | 33 ---------------- .../listener/DummyRawStoreFailEvent.java | 5 +++ .../listener/TestDbNotificationListener.java | 1 + .../hadoop/hive/metastore/HiveMetaStore.java | 40 ++++++++++--------- .../MetaStoreEventListenerConstants.java | 41 ++++++++++++++++++++ .../metastore/MetaStoreListenerNotifier.java | 15 ++++++- .../apache/hadoop/hive/metastore/RawStore.java | 2 + .../hadoop/hive/metastore/hbase/HBaseStore.java | 5 +++ .../DummyRawStoreControlledCommit.java | 5 +++ .../DummyRawStoreForJdoConnection.java | 5 +++ 11 files changed, 100 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java ---------------------------------------------------------------------- diff --git a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java b/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java index bbfbc36..f08b970 100644 --- a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java +++ b/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java @@ -30,6 +30,7 @@ import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStore.HMSHandler; import org.apache.hadoop.hive.metastore.MetaStoreEventListener; +import org.apache.hadoop.hive.metastore.MetaStoreEventListenerConstants; import org.apache.hadoop.hive.metastore.RawStore; import org.apache.hadoop.hive.metastore.RawStoreProxy; import org.apache.hadoop.hive.metastore.ReplChangeManager; http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java ---------------------------------------------------------------------- diff --git a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java b/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java deleted file mode 100644 index a4f2d59..0000000 --- a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/MetaStoreEventListenerConstants.java +++ /dev/null @@ -1,33 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hive.hcatalog.listener; - -/** - * Keeps a list of reserved keys used by Hive listeners when updating the ListenerEvent - * parameters. - */ -public class MetaStoreEventListenerConstants { - /* - * DbNotificationListener keys reserved for updating ListenerEvent parameters. - * - * DB_NOTIFICATION_EVENT_ID_KEY_NAME This key will have the event identifier that DbNotificationListener - * processed during an event. This event identifier might be shared - * across other MetaStoreEventListener implementations. - */ - public static final String DB_NOTIFICATION_EVENT_ID_KEY_NAME = "DB_NOTIFICATION_EVENT_ID_KEY_NAME"; -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java ---------------------------------------------------------------------- diff --git a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java index 5282a5a..74d0efe 100644 --- a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java +++ b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java @@ -93,6 +93,11 @@ public class DummyRawStoreFailEvent implements RawStore, Configurable { } @Override + public boolean isActiveTransaction() { + return false; + } + + @Override public Configuration getConf() { return objectStore.getConf(); } http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/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 50d8878..976c3c5 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 @@ -41,6 +41,7 @@ import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; import org.apache.hadoop.hive.metastore.IMetaStoreClient; import org.apache.hadoop.hive.metastore.MetaStoreEventListener; +import org.apache.hadoop.hive.metastore.MetaStoreEventListenerConstants; import org.apache.hadoop.hive.metastore.api.Database; import org.apache.hadoop.hive.metastore.api.FieldSchema; import org.apache.hadoop.hive.metastore.api.FireEventRequest; http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java index c4e45a1..6f2b727 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java @@ -904,7 +904,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.CREATE_DATABASE, new CreateDatabaseEvent(db, success, this), null, - transactionalListenersResponses); + transactionalListenersResponses, ms); } } } @@ -1136,7 +1136,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_DATABASE, new DropDatabaseEvent(db, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -1477,7 +1477,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.CREATE_TABLE, new CreateTableEvent(tbl, success, this), envContext, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -1722,7 +1722,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_TABLE, new DropTableEvent(tbl, deleteData, success, this), envContext, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } return success; @@ -2266,7 +2266,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ADD_PARTITION, new AddPartitionEvent(tbl, part, success, this), envContext, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } return part; @@ -2521,7 +2521,8 @@ public class HiveMetaStore extends ThriftHiveMetastore { if (!listeners.isEmpty()) { MetaStoreListenerNotifier.notifyEvent(listeners, EventType.ADD_PARTITION, - new AddPartitionEvent(tbl, parts, false, this)); + new AddPartitionEvent(tbl, parts, false, this), + null, null, ms); } } else { if (!listeners.isEmpty()) { @@ -2529,13 +2530,14 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ADD_PARTITION, new AddPartitionEvent(tbl, newParts, true, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); if (!existingParts.isEmpty()) { // The request has succeeded but we failed to add these partitions. MetaStoreListenerNotifier.notifyEvent(listeners, EventType.ADD_PARTITION, - new AddPartitionEvent(tbl, existingParts, false, this)); + new AddPartitionEvent(tbl, existingParts, false, this), + null, null, ms); } } } @@ -2723,7 +2725,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ADD_PARTITION, new AddPartitionEvent(tbl, partitionSpecProxy, true, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -2877,7 +2879,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ADD_PARTITION, new AddPartitionEvent(tbl, Arrays.asList(part), success, this), envContext, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } @@ -3031,7 +3033,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ADD_PARTITION, addPartitionEvent, null, - transactionalListenerResponsesForAddPartition); + transactionalListenerResponsesForAddPartition, ms); i = 0; for (Partition partition : partitionsToExchange) { @@ -3046,7 +3048,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_PARTITION, dropPartitionEvent, null, - parameters); + parameters, ms); i++; } } @@ -3137,7 +3139,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_PARTITION, new DropPartitionEvent(tbl, part, success, deleteData, this), envContext, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } return true; @@ -3334,7 +3336,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_PARTITION, new DropPartitionEvent(tbl, part, success, deleteData, this), envContext, - parameters); + parameters, ms); i++; } @@ -3926,7 +3928,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.ALTER_INDEX, new AlterIndexEvent(oldIndex, newIndex, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -4629,7 +4631,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.CREATE_INDEX, new AddIndexEvent(index, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -4722,7 +4724,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_INDEX, new DropIndexEvent(index, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } return success; @@ -6193,7 +6195,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.CREATE_FUNCTION, new CreateFunctionEvent(func, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } @@ -6232,7 +6234,7 @@ public class HiveMetaStore extends ThriftHiveMetastore { EventType.DROP_FUNCTION, new DropFunctionEvent(func, success, this), null, - transactionalListenerResponses); + transactionalListenerResponses, ms); } } } http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListenerConstants.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListenerConstants.java b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListenerConstants.java new file mode 100644 index 0000000..79de79d --- /dev/null +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListenerConstants.java @@ -0,0 +1,41 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore; + +/** + * Keeps a list of reserved keys used by Hive listeners when updating the ListenerEvent + * parameters. + */ +public class MetaStoreEventListenerConstants { + /* + * DbNotificationListener keys reserved for updating ListenerEvent parameters. + * + * DB_NOTIFICATION_EVENT_ID_KEY_NAME This key will have the event identifier that DbNotificationListener + * processed during an event. This event identifier might be shared + * across other MetaStoreEventListener implementations. + */ + public static final String DB_NOTIFICATION_EVENT_ID_KEY_NAME = "DB_NOTIFICATION_EVENT_ID_KEY_NAME"; + + /* + * HiveMetaStore keys reserved for updating ListenerEvent parameters. + * + * HIVE_METASTORE_TRANSACTION_ACTIVE This key is used to check if a listener event is run inside a current + * transaction. A boolean value is used for active (true) or no active (false). + */ + public static final String HIVE_METASTORE_TRANSACTION_ACTIVE = "HIVE_METASTORE_TRANSACTION_ACTIVE"; +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java index 20011cc..37327f8 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListenerNotifier.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hive.metastore.events.ListenerEvent; import java.util.List; import java.util.Map; +import static org.apache.hadoop.hive.metastore.MetaStoreEventListenerConstants.HIVE_METASTORE_TRANSACTION_ACTIVE; import static org.apache.hadoop.hive.metastore.messaging.EventMessage.EventType; /** @@ -201,11 +202,17 @@ public class MetaStoreListenerNotifier { * the (ListenerEvent) event by setting a parameter key/value pair. These updated parameters will * be returned to the caller. * + * Sometimes these events are run inside a DB transaction and might cause issues with the listeners, + * for instance, Sentry blocks the HMS until an event is seen committed on the DB. To notify the listener about this, + * a new parameter to verify if a transaction is active is added to the ListenerEvent, and is up to the listener + * to skip this notification if so. + * * @param listeners List of MetaStoreEventListener listeners. * @param eventType Type of the notification event. * @param event The ListenerEvent with information about the event. * @param environmentContext An EnvironmentContext object with parameters sent by the HMS client. * @param parameters A list of key/value pairs with the new parameters to add. + * @param ms The RawStore object from where to check if a transaction is active. * @return A list of key/value pair parameters that the listeners set. The returned object will return an empty * map if no parameters were updated or if no listeners were notified. * @throws MetaException If an error occurred while calling the listeners. @@ -214,11 +221,17 @@ public class MetaStoreListenerNotifier { EventType eventType, ListenerEvent event, EnvironmentContext environmentContext, - Map<String, String> parameters) throws MetaException { + Map<String, String> parameters, + final RawStore ms) throws MetaException { Preconditions.checkNotNull(event, "The event must not be null."); event.putParameters(parameters); + + if (ms != null) { + event.putParameter(HIVE_METASTORE_TRANSACTION_ACTIVE, Boolean.toString(ms.isActiveTransaction())); + } + return notifyEvent(listeners, eventType, event, environmentContext); } } http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java index 6f4f031..5b40835 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java @@ -93,6 +93,8 @@ public interface RawStore extends Configurable { @CanNotRetry public abstract boolean commitTransaction(); + public boolean isActiveTransaction(); + /** * Rolls back the current transaction if it is active */ http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java index 6593fa6..ecddb8a 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java @@ -139,6 +139,11 @@ public class HBaseStore implements RawStore { } @Override + public boolean isActiveTransaction() { + return txnNestLevel != 0; + } + + @Override public void rollbackTransaction() { txnNestLevel = 0; LOG.debug("Rolling back HBase transaction"); http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java ---------------------------------------------------------------------- diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java index f64b08d..275797e 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java @@ -95,6 +95,11 @@ public class DummyRawStoreControlledCommit implements RawStore, Configurable { } } + @Override + public boolean isActiveTransaction() { + return false; + } + // All remaining functions simply delegate to objectStore @Override http://git-wip-us.apache.org/repos/asf/hive/blob/cd39cf38/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java ---------------------------------------------------------------------- diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java index 2682886..7f1784e 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java @@ -109,6 +109,11 @@ public class DummyRawStoreForJdoConnection implements RawStore { } @Override + public boolean isActiveTransaction() { + return false; + } + + @Override public void rollbackTransaction() {
