This is an automated email from the ASF dual-hosted git repository.
dengzh 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 7b5433415e4 HIVE-29409: Async request fails to retry on transparent
exceptions (#6284)
7b5433415e4 is described below
commit 7b5433415e42ec92d29636109ea7f522c9354cf4
Author: dengzh <[email protected]>
AuthorDate: Thu Feb 5 08:08:50 2026 +0800
HIVE-29409: Async request fails to retry on transparent exceptions (#6284)
---
.../client/ThriftHiveMetaStoreClient.java | 2 +
standalone-metastore/metastore-server/pom.xml | 1 -
.../apache/hadoop/hive/metastore/HMSHandler.java | 10 +
.../hive/metastore/handler/CreateTableHandler.java | 12 +-
.../metastore/handler/DropDatabaseHandler.java | 40 ++--
.../metastore/handler/TestRetryOnException.java | 211 +++++++++++++++++++++
standalone-metastore/pom.xml | 1 +
7 files changed, 252 insertions(+), 25 deletions(-)
diff --git
a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java
b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java
index 50d9fae5963..563a4a8c99d 100644
---
a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java
+++
b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java
@@ -1530,6 +1530,7 @@ public void dropDatabase(DropDatabaseRequest req) throws
TException {
try {
while (!resp.isFinished() && !Thread.currentThread().isInterrupted()) {
resp = client.drop_database_req(req);
+ req.setId(resp.getId());
if (resp.getMessage() != null) {
LOG.info(resp.getMessage());
}
@@ -1706,6 +1707,7 @@ public void dropTable(String catName, String dbname,
String name, boolean delete
try {
while (!resp.isFinished() && !Thread.currentThread().isInterrupted()) {
resp = client.drop_table_req(dropTableReq);
+ dropTableReq.setId(resp.getId());
if (resp.getMessage() != null) {
LOG.info(resp.getMessage());
}
diff --git a/standalone-metastore/metastore-server/pom.xml
b/standalone-metastore/metastore-server/pom.xml
index 47783d74f7f..64926eaba1c 100644
--- a/standalone-metastore/metastore-server/pom.xml
+++ b/standalone-metastore/metastore-server/pom.xml
@@ -23,7 +23,6 @@
<name>Hive Metastore Server</name>
<properties>
<standalone.metastore.path.to.root>..</standalone.metastore.path.to.root>
- <reflections.version>0.10.2</reflections.version>
</properties>
<dependencies>
<dependency>
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
index 903fd631962..e6cc721b153 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
@@ -1587,6 +1587,10 @@ public AsyncOperationResp drop_database_req(final
DropDatabaseRequest req)
return status.toAsyncOperationResp();
} catch (Exception e) {
ex = e;
+ // Reset the id of the request in case of RetryingHMSHandler retries
+ if (req.getId() != null && req.isAsyncDrop() && !req.isCancel()) {
+ req.setId(null);
+ }
throw handleException(e)
.throwIfInstance(NoSuchObjectException.class,
InvalidOperationException.class, MetaException.class)
.defaultMetaException();
@@ -2429,6 +2433,12 @@ public AsyncOperationResp
drop_table_req(DropTableRequest dropReq)
return status.toAsyncOperationResp();
} catch (Exception e) {
ex = e;
+ // Here we get an exception, the RetryingHMSHandler might retry the call,
+ // need to clear the id from the request, so AbstractRequestHandler can
+ // start a new execution other than reuse the cached failed handler.
+ if (dropReq.getId() != null && dropReq.isAsyncDrop() &&
!dropReq.isCancel()) {
+ dropReq.setId(null);
+ }
throw handleException(e).throwIfInstance(MetaException.class,
NoSuchObjectException.class)
.convertIfInstance(IOException.class,
MetaException.class).defaultMetaException();
} finally {
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateTableHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateTableHandler.java
index 35438f3b466..81da6dc7eb2 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateTableHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateTableHandler.java
@@ -413,27 +413,27 @@ protected void beforeExecute() throws TException,
IOException {
String catName = tbl.getCatName();
// Check that constraints have catalog name properly set first
if (CollectionUtils.isNotEmpty(constraints.getPrimaryKeys())
- && !constraints.getPrimaryKeys().get(0).isSetCatName()) {
+ && !constraints.getPrimaryKeys().getFirst().isSetCatName()) {
constraints.getPrimaryKeys().forEach(constraint ->
constraint.setCatName(catName));
}
if (CollectionUtils.isNotEmpty(constraints.getForeignKeys())
- && !constraints.getForeignKeys().get(0).isSetCatName()) {
+ && !constraints.getForeignKeys().getFirst().isSetCatName()) {
constraints.getForeignKeys().forEach(constraint ->
constraint.setCatName(catName));
}
if (CollectionUtils.isNotEmpty(constraints.getUniqueConstraints())
- && !constraints.getUniqueConstraints().get(0).isSetCatName()) {
+ && !constraints.getUniqueConstraints().getFirst().isSetCatName()) {
constraints.getUniqueConstraints().forEach(constraint ->
constraint.setCatName(catName));
}
if (CollectionUtils.isNotEmpty(constraints.getNotNullConstraints())
- && !constraints.getNotNullConstraints().get(0).isSetCatName()) {
+ && !constraints.getNotNullConstraints().getFirst().isSetCatName()) {
constraints.getNotNullConstraints().forEach(constraint ->
constraint.setCatName(catName));
}
if (CollectionUtils.isNotEmpty(constraints.getDefaultConstraints())
- && !constraints.getDefaultConstraints().get(0).isSetCatName()) {
+ && !constraints.getDefaultConstraints().getFirst().isSetCatName()) {
constraints.getDefaultConstraints().forEach(constraint ->
constraint.setCatName(catName));
}
if (CollectionUtils.isNotEmpty(constraints.getCheckConstraints())
- && !constraints.getCheckConstraints().get(0).isSetCatName()) {
+ && !constraints.getCheckConstraints().getFirst().isSetCatName()) {
constraints.getCheckConstraints().forEach(constraint ->
constraint.setCatName(catName));
}
}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java
index fb372c8b8de..3a475040a7f 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java
@@ -212,24 +212,7 @@ protected void beforeExecute() throws TException,
IOException {
pkgRequest.setDbName(name);
packages = defaultEmptyList(rs.listPackages(pkgRequest));
- if (!request.isCascade()) {
- if (!tableNames.isEmpty()) {
- throw new InvalidOperationException(
- "Database " + db.getName() + " is not empty. One or more tables
exist.");
- }
- if (!functions.isEmpty()) {
- throw new InvalidOperationException(
- "Database " + db.getName() + " is not empty. One or more functions
exist.");
- }
- if (!procedures.isEmpty()) {
- throw new InvalidOperationException(
- "Database " + db.getName() + " is not empty. One or more stored
procedures exist.");
- }
- if (!packages.isEmpty()) {
- throw new InvalidOperationException(
- "Database " + db.getName() + " is not empty. One or more packages
exist.");
- }
- }
+ validateCascadeDrop(tableNames);
Path path = new Path(db.getLocationUri()).getParent();
if (!handler.getWh().isWritable(path)) {
throw new MetaException("Database not dropped since its external
warehouse location " + path +
@@ -271,6 +254,27 @@ private void checkFuncPathToCm() {
result.setFunctionCmPaths(funcNeedCmPaths);
}
+ private void validateCascadeDrop(List<String> tableNames) throws
InvalidOperationException {
+ if (!request.isCascade()) {
+ if (!tableNames.isEmpty()) {
+ throw new InvalidOperationException(
+ "Database " + db.getName() + " is not empty. One or more tables
exist.");
+ }
+ if (!functions.isEmpty()) {
+ throw new InvalidOperationException(
+ "Database " + db.getName() + " is not empty. One or more functions
exist.");
+ }
+ if (!procedures.isEmpty()) {
+ throw new InvalidOperationException(
+ "Database " + db.getName() + " is not empty. One or more stored
procedures exist.");
+ }
+ if (!packages.isEmpty()) {
+ throw new InvalidOperationException(
+ "Database " + db.getName() + " is not empty. One or more packages
exist.");
+ }
+ }
+ }
+
private void checkTablePathPermission(RawStore rs, List<String> tableNames)
throws MetaException {
int tableBatchSize = MetastoreConf.getIntVar(handler.getConf(),
MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX);
Warehouse wh = handler.getWh();
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestRetryOnException.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestRetryOnException.java
new file mode 100644
index 00000000000..c543dfaec23
--- /dev/null
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestRetryOnException.java
@@ -0,0 +1,211 @@
+/*
+ * 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.handler;
+
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
+import org.apache.hadoop.hive.metastore.TableType;
+import org.apache.hadoop.hive.metastore.TransactionalMetaStoreEventListener;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreCheckinTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.MetaStoreClientTest;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.events.DropDatabaseEvent;
+import org.apache.hadoop.hive.metastore.events.DropTableEvent;
+import org.apache.hadoop.hive.metastore.minihms.AbstractMetaStoreService;
+import org.apache.thrift.TException;
+import org.datanucleus.exceptions.NucleusException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+@RunWith(Parameterized.class)
+@Category(MetastoreCheckinTest.class)
+public class TestRetryOnException extends MetaStoreClientTest {
+ protected static final String DB_NAME = "testdroptable";
+ protected static final String TABLE_NAME = "test_drop_table";
+ private final String testTempDir =
+ Paths.get(System.getProperty("java.io.tmpdir"),
"testDropTable").toString();
+ private AbstractMetaStoreService metaStore;
+ private HiveMetaStoreClient client;
+
+ public TestRetryOnException(String name, AbstractMetaStoreService metaStore)
{
+ this.metaStore = metaStore;
+ }
+
+ @BeforeClass
+ public static void startMetaStores() {
+ Map<MetastoreConf.ConfVars, String> msConf = new HashMap<>();
+ Map<String, String> extraConf = new HashMap<>();
+ extraConf.put("metastore.transactional.event.listeners",
AssertExTransactionListener.class.getName());
+ startMetaStores(msConf, extraConf);
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ AssertExTransactionListener.resetTest();
+ client = metaStore.getClient();
+ cleanDB();
+ createDB(DB_NAME);
+ AssertExTransactionListener.startTest();
+ }
+
+ @Test
+ public void testRetryOnException() throws Exception {
+ String location = metaStore.getExternalWarehouseRoot() + "/" + TABLE_NAME;
+ new TableBuilder()
+ .setDbName(DB_NAME)
+ .setTableName(TABLE_NAME)
+ .addCol("test_id", "int", "test col id")
+ .addCol("test_value", "string", "test col value")
+ .addTableParam("param_key", "param_value")
+ .setType(TableType.EXTERNAL_TABLE.name())
+ .addTableParam("EXTERNAL", "TRUE")
+ .addStorageDescriptorParam("sd_param_key", "sd_param_value")
+ .setSerdeName(TABLE_NAME)
+ .setStoredAsSubDirectories(false)
+ .addSerdeParam("serde_param_key", "serde_param_value")
+ .setLocation(location)
+ .create(client, metaStore.getConf());
+
+ Table table = client.getTable(DB_NAME, TABLE_NAME);
+ assertNotNull(table);
+ String tableName = TableName.getQualified(table.getCatName(),
table.getDbName(), table.getTableName());
+
+ assertEquals(0, AssertExTransactionListener.getCalls(tableName));
+ client.dropTable(DB_NAME, TABLE_NAME);
+ assertEquals(AssertExTransactionListener.maxRetries,
AssertExTransactionListener.getCalls(tableName));
+ try {
+ client.getTable(DB_NAME, TABLE_NAME);
+ fail("Table " + tableName + " should be dropped");
+ } catch (NoSuchObjectException nse) {
+ // expected
+ }
+
+ assertEquals(0, AssertExTransactionListener.getCalls(DB_NAME));
+ client.dropDatabase(DB_NAME);
+ assertEquals(AssertExTransactionListener.maxRetries,
AssertExTransactionListener.getCalls(tableName));
+ try {
+ client.getDatabase(DB_NAME);
+ fail("Database " + DB_NAME + " should be dropped");
+ } catch (NoSuchObjectException nse) {
+ // expected
+ }
+ }
+
+ public static class AssertExTransactionListener extends
TransactionalMetaStoreEventListener {
+ static Map<String, Integer> calls = new HashMap<>();
+ static int maxRetries = 5;
+ static boolean startChecking = false;
+
+ public AssertExTransactionListener(Configuration config) {
+ super(config);
+ }
+
+ @Override
+ public void onDropTable(DropTableEvent tableEvent) throws MetaException {
+ Table table = tableEvent.getTable();
+ String tableName = TableName.getQualified(table.getCatName(),
table.getDbName(), table.getTableName());
+ Integer pre = calls.putIfAbsent(tableName, 1);
+ if (pre != null) {
+ calls.put(tableName, pre + 1);
+ }
+ // For a drop table call, we should see 5(maxRetries) retries in
RetryingHMSHandler
+ assertException(startChecking && calls.get(tableName) < maxRetries);
+ }
+
+ @Override
+ public void onDropDatabase(DropDatabaseEvent dbEvent) throws MetaException
{
+ String dbName = dbEvent.getDatabase().getName();
+ Integer pre = calls.putIfAbsent(dbName, 1);
+ if (pre != null) {
+ calls.put(dbName, pre + 1);
+ }
+ assertException(startChecking && calls.get(dbName) < maxRetries);
+ }
+
+ private void assertException(boolean throwException) throws MetaException {
+ if (throwException) {
+ MetaException exception = new MetaException("An exception for
RetryingHMSHandler to retry");
+ exception.initCause(new NucleusException("Non-fatal exception"));
+ throw exception;
+ }
+ }
+
+ static void startTest() {
+ startChecking = true;
+ calls.clear();
+ }
+
+ static void resetTest() {
+ startChecking = false;
+ calls.clear();
+ }
+
+ static int getCalls(String key) {
+ return calls.get(key) != null ? calls.get(key) : 0;
+ }
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ try {
+ if (client != null) {
+ try {
+ client.close();
+ } catch (Exception e) {
+ }
+ }
+ } finally {
+ client = null;
+ }
+
+ Path path = new Path(testTempDir);
+ path.getFileSystem(metaStore.getConf()).delete(path, true);
+ }
+
+ protected void cleanDB() throws Exception{
+ client.dropDatabase(DB_NAME, true, true, true);
+ metaStore.cleanWarehouseDirs();
+ }
+
+ protected void createDB(String dbName) throws TException {
+ new DatabaseBuilder().
+ setName(dbName).
+ create(client, metaStore.getConf());
+ }
+}
diff --git a/standalone-metastore/pom.xml b/standalone-metastore/pom.xml
index 8c9f5256f91..30ca1bf00f4 100644
--- a/standalone-metastore/pom.xml
+++ b/standalone-metastore/pom.xml
@@ -101,6 +101,7 @@
<protobuf.version>3.25.5</protobuf.version>
<protoc-jar-maven-plugin.version>3.11.4</protoc-jar-maven-plugin.version>
<protoc.path>${env.PROTOC_PATH}</protoc.path>
+ <reflections.version>0.10.2</reflections.version>
<io.grpc.version>1.72.0</io.grpc.version>
<sqlline.version>1.9.0</sqlline.version>
<netty.version>4.1.127.Final</netty.version>
