This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 96ed10dc6f [Cherry-pick to branch-1.3] [#13020] fix(hive): Skip HMS
stats update for property/comment-only alterTable (#13021) (#13074)
96ed10dc6f is described below
commit 96ed10dc6f8a96608819e40ecec8d7068a7abcfb
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 20:05:08 2026 +0800
[Cherry-pick to branch-1.3] [#13020] fix(hive): Skip HMS stats update for
property/comment-only alterTable (#13021) (#13074)
**Cherry-pick Information:**
- Original commit: 0e6ae74f7f0b34ccb06e0058e69b929d278e2d5c
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: geyanggang <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
---
.../catalog/hive/HiveCatalogOperations.java | 34 +++++-
.../catalog/hive/TestHiveCatalogOperations.java | 44 ++++++++
.../apache/gravitino/hive/client/HiveClient.java | 27 ++++-
.../gravitino/hive/client/HiveClientImpl.java | 8 +-
.../org/apache/gravitino/hive/client/HiveShim.java | 6 +-
.../apache/gravitino/hive/client/HiveShimV2.java | 30 +++++-
.../apache/gravitino/hive/client/HiveShimV3.java | 46 +++++++--
.../apache/gravitino/hive/client/TestHive2HMS.java | 4 +-
.../hive/client/TestHiveShimAlterTable.java | 115 +++++++++++++++++++++
9 files changed, 297 insertions(+), 17 deletions(-)
diff --git
a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogOperations.java
b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogOperations.java
index d2e17aa6c7..cd73a9eed4 100644
---
a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogOperations.java
+++
b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogOperations.java
@@ -741,9 +741,18 @@ public class HiveCatalogOperations
targetDatabaseName);
HiveTable finalUpdatedTable = updatedTable;
+ // For property-only or comment-only changes, skip the metastore
statistics recomputation so
+ // it does not access the table's storage location. This keeps such
lightweight alters from
+ // hanging when the underlying filesystem (e.g. HDFS NameNode) is slow
or unavailable.
+ boolean skipStatsUpdate = canSkipStatsUpdate(changes);
clientPool.run(
c -> {
- c.alterTable(catalogName, schemaIdent.name(), tableIdent.name(),
finalUpdatedTable);
+ c.alterTable(
+ catalogName,
+ schemaIdent.name(),
+ tableIdent.name(),
+ finalUpdatedTable,
+ skipStatsUpdate);
return null;
});
@@ -767,6 +776,29 @@ public class HiveCatalogOperations
}
}
+ /**
+ * Determines whether the metastore statistics recomputation can be skipped
for the given table
+ * changes. Statistics are tied to the table data, so recomputation is only
meaningful when the
+ * data layout may change. Property-only and comment-only alters never touch
the data, so they can
+ * safely skip the recomputation (and the storage-location access it
triggers). Any column change
+ * or rename falls back to the default behavior.
+ *
+ * @param changes The table changes to be applied.
+ * @return {@code true} if every change is a property or comment change;
{@code false} otherwise.
+ */
+ @VisibleForTesting
+ static boolean canSkipStatsUpdate(TableChange[] changes) {
+ if (changes == null || changes.length == 0) {
+ return false;
+ }
+ return Arrays.stream(changes)
+ .allMatch(
+ change ->
+ change instanceof TableChange.SetProperty
+ || change instanceof TableChange.RemoveProperty
+ || change instanceof TableChange.UpdateComment);
+ }
+
private HiveTable buildAlteredHiveTable(
HiveTable original,
String tableName,
diff --git
a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalogOperations.java
b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalogOperations.java
index 832ec7fc67..5b7b6834bf 100644
---
a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalogOperations.java
+++
b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalogOperations.java
@@ -68,6 +68,7 @@ import org.apache.gravitino.hive.client.HiveClient;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.Representation;
import org.apache.gravitino.rel.SQLRepresentation;
+import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.View;
import org.apache.gravitino.rel.ViewChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
@@ -1001,4 +1002,47 @@ class TestHiveCatalogOperations {
boolean dropped = op.dropView(NameIdentifier.of("db", "t1"));
Assertions.assertFalse(dropped);
}
+
+ @Test
+ void testCanSkipStatsUpdate() {
+ // Property-only and comment-only changes can skip the metastore
statistics recomputation.
+ Assertions.assertTrue(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.setProperty("k", "v")}));
+ Assertions.assertTrue(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.removeProperty("k")}));
+ Assertions.assertTrue(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.updateComment("new comment")}));
+ Assertions.assertTrue(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {
+ TableChange.setProperty("k", "v"),
+ TableChange.removeProperty("k2"),
+ TableChange.updateComment("c")
+ }));
+
+ // Column changes and renames must not skip the statistics recomputation.
+ Assertions.assertFalse(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.addColumn(new String[] {"c"},
Types.StringType.get())}));
+ Assertions.assertFalse(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.deleteColumn(new String[] {"c"},
true)}));
+ Assertions.assertFalse(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {TableChange.rename("newName")}));
+ // A mix that contains a column change falls back to the default behavior.
+ Assertions.assertFalse(
+ HiveCatalogOperations.canSkipStatsUpdate(
+ new TableChange[] {
+ TableChange.setProperty("k", "v"),
+ TableChange.addColumn(new String[] {"c"}, Types.StringType.get())
+ }));
+
+ // No changes: nothing to optimize, keep the default behavior.
+ Assertions.assertFalse(HiveCatalogOperations.canSkipStatsUpdate(new
TableChange[] {}));
+ Assertions.assertFalse(HiveCatalogOperations.canSkipStatsUpdate(null));
+ }
}
diff --git
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClient.java
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClient.java
index ab83ce4ea0..ec52dc6485 100644
---
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClient.java
+++
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClient.java
@@ -51,8 +51,33 @@ public interface HiveClient extends AutoCloseable {
HiveTable getTable(String catalogName, String databaseName, String
tableName);
+ default void alterTable(
+ String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable) {
+ alterTable(catalogName, databaseName, tableName, alteredHiveTable, false);
+ }
+
+ /**
+ * Alters a table in the Hive metastore.
+ *
+ * <p>When {@code skipStatsUpdate} is {@code true}, the metastore is
instructed (via the {@code
+ * DO_NOT_UPDATE_STATS} environment context) not to recompute table
statistics for this alter.
+ * This avoids the metastore accessing the table's storage location (for
example an {@code
+ * getFileInfo} call against the NameNode), which is unnecessary for
property-only or comment-only
+ * changes and can otherwise make a lightweight alter hang when the
underlying filesystem is slow
+ * or unavailable.
+ *
+ * @param catalogName The Hive catalog name.
+ * @param databaseName The database name.
+ * @param tableName The table name.
+ * @param alteredHiveTable The altered table definition.
+ * @param skipStatsUpdate Whether to skip metastore statistics recomputation
for this alter.
+ */
void alterTable(
- String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable);
+ String catalogName,
+ String databaseName,
+ String tableName,
+ HiveTable alteredHiveTable,
+ boolean skipStatsUpdate);
void dropTable(
String catalogName,
diff --git
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientImpl.java
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientImpl.java
index 25b32cef44..550d730567 100644
---
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientImpl.java
+++
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientImpl.java
@@ -99,8 +99,12 @@ public class HiveClientImpl implements HiveClient {
@Override
public void alterTable(
- String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable) {
- shim.alterTable(catalogName, databaseName, tableName, alteredHiveTable);
+ String catalogName,
+ String databaseName,
+ String tableName,
+ HiveTable alteredHiveTable,
+ boolean skipStatsUpdate) {
+ shim.alterTable(catalogName, databaseName, tableName, alteredHiveTable,
skipStatsUpdate);
}
@Override
diff --git
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java
index e346d328bf..7dbdca1426 100644
---
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java
+++
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java
@@ -79,7 +79,11 @@ public abstract class HiveShim {
public abstract HiveTable getTable(String catalogName, String databaseName,
String tableName);
public abstract void alterTable(
- String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable);
+ String catalogName,
+ String databaseName,
+ String tableName,
+ HiveTable alteredHiveTable,
+ boolean skipStatsUpdate);
public abstract void dropTable(
String catalogName,
diff --git
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV2.java
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV2.java
index dc3fbe2c91..97b7586d51 100644
---
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV2.java
+++
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV2.java
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.hive.client.Util.updateConfigurationFromPrope
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
+import java.util.Collections;
import java.util.List;
import java.util.Properties;
import org.apache.gravitino.hive.HivePartition;
@@ -31,9 +32,11 @@ import
org.apache.gravitino.hive.client.HiveExceptionConverter.ExceptionTarget;
import org.apache.gravitino.hive.converter.HiveDatabaseConverter;
import org.apache.gravitino.hive.converter.HiveTableConverter;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.StatsSetupConst;
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
import org.apache.hadoop.hive.metastore.TableType;
import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
class HiveShimV2 extends HiveShim {
@@ -148,10 +151,22 @@ class HiveShimV2 extends HiveShim {
@Override
public void alterTable(
- String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable) {
+ String catalogName,
+ String databaseName,
+ String tableName,
+ HiveTable alteredHiveTable,
+ boolean skipStatsUpdate) {
try {
var tb = HiveTableConverter.toHiveTable(alteredHiveTable);
- client.alter_table(databaseName, tableName, tb);
+ if (skipStatsUpdate) {
+ // Instruct the metastore not to recompute statistics for this alter,
so it does not access
+ // the table's storage location. Hive 2.x has no catalog-aware alter,
so the database name
+ // is used directly.
+ client.alter_table_with_environmentContext(
+ databaseName, tableName, tb, doNotUpdateStatsContext());
+ } else {
+ client.alter_table(databaseName, tableName, tb);
+ }
} catch (Exception e) {
throw HiveExceptionConverter.toGravitinoException(e,
ExceptionTarget.table(tableName));
}
@@ -294,4 +309,15 @@ class HiveShimV2 extends HiveShim {
public void close() throws Exception {
client.close();
}
+
+ /**
+ * Builds an {@link EnvironmentContext} that tells the metastore not to
recompute table statistics
+ * during an alter, avoiding an access to the table's storage location.
+ *
+ * @return An environment context with {@code DO_NOT_UPDATE_STATS} set to
{@code true}.
+ */
+ protected EnvironmentContext doNotUpdateStatsContext() {
+ return new EnvironmentContext(
+ Collections.singletonMap(StatsSetupConst.DO_NOT_UPDATE_STATS,
StatsSetupConst.TRUE));
+ }
}
diff --git
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV3.java
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV3.java
index 1d6fca6ed9..da6400991c 100644
---
a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV3.java
+++
b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShimV3.java
@@ -36,6 +36,7 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
import org.apache.hadoop.hive.metastore.TableType;
import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
import org.apache.hadoop.hive.metastore.api.Table;
class HiveShimV3 extends HiveShimV2 {
@@ -50,6 +51,7 @@ class HiveShimV3 extends HiveShimV2 {
private final Method getTableMethod;
private final Method createTableMethod;
private final Method alterTableMethod;
+ private final Method alterTableWithEnvironmentContextMethod;
private final Method dropTableMethod;
private final Method getAllTablesMethod;
private final Method getTablesByTypeMethod;
@@ -107,6 +109,14 @@ class HiveShimV3 extends HiveShimV2 {
String.class,
String.class,
org.apache.hadoop.hive.metastore.api.Table.class);
+ this.alterTableWithEnvironmentContextMethod =
+ IMetaStoreClient.class.getMethod(
+ "alter_table",
+ String.class,
+ String.class,
+ String.class,
+ Table.class,
+ EnvironmentContext.class);
this.dropTableMethod =
IMetaStoreClient.class.getMethod(
"dropTable", String.class, String.class, String.class,
boolean.class, boolean.class);
@@ -293,17 +303,35 @@ class HiveShimV3 extends HiveShimV2 {
@Override
public void alterTable(
- String catalogName, String databaseName, String tableName, HiveTable
alteredHiveTable) {
+ String catalogName,
+ String databaseName,
+ String tableName,
+ HiveTable alteredHiveTable,
+ boolean skipStatsUpdate) {
var tb = HiveTableConverter.toHiveTable(alteredHiveTable);
invoke(ExceptionTarget.other(""), tb, tableSetCatalogNameMethod,
catalogName);
- invoke(
- ExceptionTarget.table(tableName),
- client,
- alterTableMethod,
- catalogName,
- databaseName,
- tableName,
- tb);
+ if (skipStatsUpdate) {
+ // Instruct the metastore not to recompute statistics for this alter, so
it does not access
+ // the table's storage location.
+ invoke(
+ ExceptionTarget.table(tableName),
+ client,
+ alterTableWithEnvironmentContextMethod,
+ catalogName,
+ databaseName,
+ tableName,
+ tb,
+ doNotUpdateStatsContext());
+ } else {
+ invoke(
+ ExceptionTarget.table(tableName),
+ client,
+ alterTableMethod,
+ catalogName,
+ databaseName,
+ tableName,
+ tb);
+ }
}
@Override
diff --git
a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHive2HMS.java
b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHive2HMS.java
index 6dbc9d5edd..418ccbfe6d 100644
---
a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHive2HMS.java
+++
b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHive2HMS.java
@@ -148,7 +148,9 @@ public class TestHive2HMS {
Assertions.assertEquals(
1, loadedTable.partitioning().length, "Table should have 1 partition
key");
- hiveClient.alterTable(catalogName, dbName, tableName, loadedTable);
+ // Use skipStatsUpdate=true so the metastore does not recompute
statistics or access the
+ // table's storage location for this property-only alter.
+ hiveClient.alterTable(catalogName, dbName, tableName, loadedTable, true);
HiveTable alteredTable = hiveClient.getTable(catalogName, dbName,
tableName);
Assertions.assertNotNull(alteredTable, "Altered table should not be
null");
diff --git
a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHiveShimAlterTable.java
b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHiveShimAlterTable.java
new file mode 100644
index 0000000000..251cccd450
--- /dev/null
+++
b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/client/TestHiveShimAlterTable.java
@@ -0,0 +1,115 @@
+/*
+ * 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.gravitino.hive.client;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import org.apache.gravitino.catalog.hive.HiveConstants;
+import org.apache.gravitino.hive.HiveTable;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.hadoop.hive.common.StatsSetupConst;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+/**
+ * Unit tests verifying that the Hive shims send a {@code DO_NOT_UPDATE_STATS}
environment context
+ * to the metastore when {@code skipStatsUpdate} is requested, and use the
plain alter otherwise.
+ */
+class TestHiveShimAlterTable {
+
+ private static final String CATALOG = "hive";
+ private static final String DB = "db";
+ private static final String TABLE = "tbl";
+
+ /**
+ * A {@link HiveShimV2} that uses a mocked metastore client instead of
connecting to a real Hive
+ * Metastore. The mock is created inside {@link
#createMetaStoreClient(Properties)} because that
+ * method is invoked from the superclass constructor, before any subclass
field is initialized.
+ */
+ private static class MockHiveShimV2 extends HiveShimV2 {
+ MockHiveShimV2() {
+ super(new Properties());
+ }
+
+ @Override
+ public IMetaStoreClient createMetaStoreClient(Properties properties) {
+ return mock(IMetaStoreClient.class);
+ }
+
+ IMetaStoreClient metaStoreClient() {
+ return client;
+ }
+ }
+
+ private HiveTable testTable() {
+ Map<String, String> properties = new HashMap<>();
+ properties.put(HiveConstants.LOCATION, "hdfs://ns/warehouse/db.db/tbl");
+ return HiveTable.builder()
+ .withName(TABLE)
+ .withColumns(new Column[] {Column.of("id", Types.IntegerType.get())})
+ .withProperties(properties)
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("tester").withCreateTime(Instant.now()).build())
+ .withCatalogName(CATALOG)
+ .withDatabaseName(DB)
+ .build();
+ }
+
+ @Test
+ void testSkipStatsUpdateSendsDoNotUpdateStats() throws Exception {
+ MockHiveShimV2 shim = new MockHiveShimV2();
+ IMetaStoreClient client = shim.metaStoreClient();
+
+ shim.alterTable(CATALOG, DB, TABLE, testTable(), true);
+
+ ArgumentCaptor<EnvironmentContext> captor =
ArgumentCaptor.forClass(EnvironmentContext.class);
+ verify(client)
+ .alter_table_with_environmentContext(eq(DB), eq(TABLE),
any(Table.class), captor.capture());
+ Assertions.assertEquals(
+ StatsSetupConst.TRUE,
+
captor.getValue().getProperties().get(StatsSetupConst.DO_NOT_UPDATE_STATS));
+ verify(client, never()).alter_table(any(), any(), any());
+ }
+
+ @Test
+ void testDefaultUsesPlainAlter() throws Exception {
+ MockHiveShimV2 shim = new MockHiveShimV2();
+ IMetaStoreClient client = shim.metaStoreClient();
+
+ shim.alterTable(CATALOG, DB, TABLE, testTable(), false);
+
+ verify(client).alter_table(eq(DB), eq(TABLE), any(Table.class));
+ verify(client, never()).alter_table_with_environmentContext(any(), any(),
any(), any());
+ }
+}
