From dc24d2e2e59503761816d13455ed91434ea8981b Mon Sep 17 00:00:00 2001 From: zdeng Date: Sat, 25 Jul 2026 19:37:26 +0800 Subject: [PATCH 1/4] HIVE-29784: Cleaning lots of expired notifications could lead to OOM --- .../hadoop/hive/metastore/ObjectStore.java | 8 +- .../metastore/iface/NotificationStore.java | 6 +- .../metastore/impl/NotificationStoreImpl.java | 31 ++- .../metastore/ExecutionContextTestUtils.java | 57 +++++ .../TestPersistenceContextEviction.java | 233 ++++++++++++++++++ 5 files changed, 319 insertions(+), 16 deletions(-) create mode 100644 standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/ExecutionContextTestUtils.java create mode 100644 standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java index 9a42ba9f01a5..f96d03a4889f 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -420,12 +420,8 @@ public T unwrap(Class iface) { if (descriptor == null) { throw new IllegalArgumentException("Unable to unwrap the store as " + iface); } - String implClassName = - conf.get("metastore." + descriptor.alias() + ".store.impl", ""); - Class ifaceImpl = descriptor.defaultImpl(); - if (StringUtils.isNotEmpty(implClassName)) { - ifaceImpl = conf.getClass(implClassName, ifaceImpl); - } + Class ifaceImpl = + conf.getClass("metastore." + descriptor.alias() + ".store.impl", descriptor.defaultImpl()); T simpl = (T) JavaUtils.newInstance(ifaceImpl); List openQueries = new LinkedList<>(); if (simpl instanceof RawStoreBundle rsb) { diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/NotificationStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/NotificationStore.java index a1d1a1db680d..7b8dbbf2af19 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/NotificationStore.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/NotificationStore.java @@ -49,10 +49,11 @@ public interface NotificationStore { void addNotificationEvent(NotificationEvent event) throws MetaException; /** - * Remove older notification events. + * Remove older notification events, transaction is explicitly handled inside. * * @param olderThan Remove any events older or equal to a given number of seconds */ + @MetaDescriptor.NoTransaction void cleanNotificationEvents(int olderThan); /** @@ -71,9 +72,10 @@ public interface NotificationStore { NotificationEventsCountResponse getNotificationEventsCount(NotificationEventsCountRequest rqst); /** - * Remove older notification events. + * Remove older notification events, transaction is explicitly handled inside. * @param olderThan Remove any events older or equal to a given number of seconds */ + @MetaDescriptor.NoTransaction void cleanWriteNotificationEvents(int olderThan); /** diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java index b7159dfda158..555caed3b7ef 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java @@ -247,16 +247,15 @@ private void cleanOlderEvents(int olderThan, Class table, String tableName) { final Optional batchSize = (eventBatchSize > 0) ? Optional.of(eventBatchSize) : Optional.empty(); final long start = System.nanoTime(); - int deleteCount = doCleanNotificationEvents(tooOld, batchSize, table, tableName); + int deleteCount = 0; + int batchCount; + do { + batchCount = cleanNotificationEventsBatch(tooOld, batchSize, table, tableName); + deleteCount += batchCount; + } while (batchCount > 0); if (deleteCount == 0) { LOG.info("No {} events found to be cleaned with eventTime < {}", tableName, tooOld); - } else { - int batchCount = 0; - do { - batchCount = doCleanNotificationEvents(tooOld, batchSize, table, tableName); - deleteCount += batchCount; - } while (batchCount > 0); } final long finish = System.nanoTime(); @@ -264,6 +263,21 @@ private void cleanOlderEvents(int olderThan, Class table, String tableName) { TimeUnit.NANOSECONDS.toMillis(finish - start)); } + private int cleanNotificationEventsBatch(final int ageSec, final Optional batchSize, + Class tableClass, String tableName) { + boolean committed = false; + baseStore.openTransaction(); + try { + int deleted = doCleanNotificationEvents(ageSec, batchSize, tableClass, tableName); + committed = baseStore.commitTransaction(); + return deleted; + } finally { + if (!committed && baseStore.isActiveTransaction()) { + baseStore.rollbackTransaction(); + } + } + } + private int doCleanNotificationEvents(final int ageSec, final Optional batchSize, Class tableClass, String tableName) { int eventsCount = 0; @@ -308,6 +322,7 @@ private int doCleanNotificationEvents(final int ageSec, final Optional rqst.getLimit()) { eventCount = rqst.getLimit(); } diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/ExecutionContextTestUtils.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/ExecutionContextTestUtils.java new file mode 100644 index 000000000000..1423788e64bf --- /dev/null +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/ExecutionContextTestUtils.java @@ -0,0 +1,57 @@ +/* + * 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; + +import org.apache.hadoop.hive.metastore.metastore.PersistenceManagerProxy; +import org.datanucleus.ExecutionContext; +import org.datanucleus.api.jdo.JDOPersistenceManager; +import org.datanucleus.cache.Level1Cache; +import org.datanucleus.state.DNStateManager; + +import javax.jdo.PersistenceManager; + +/** + * Helpers for inspecting DataNucleus L1 (persistence context) cache in unit tests. + */ +public final class ExecutionContextTestUtils { + + private ExecutionContextTestUtils() { + } + + public static ExecutionContext getExecutionContext(PersistenceManager pm) { + if (pm instanceof JDOPersistenceManager) { + return ((JDOPersistenceManager) pm).getExecutionContext(); + } + if (pm instanceof PersistenceManagerProxy.ExecutionContextReference) { + return ((PersistenceManagerProxy.ExecutionContextReference) pm).getExecutionContext(); + } + throw new IllegalArgumentException("Unsupported PersistenceManager: " + pm.getClass()); + } + + public static int countCachedInstances(PersistenceManager pm, Class clazz) { + Level1Cache l1Cache = getExecutionContext(pm).getLevel1Cache(); + int count = 0; + for (DNStateManager stateManager : l1Cache.values()) { + if (clazz.isInstance(stateManager.getObject())) { + count++; + } + } + return count; + } +} diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java new file mode 100644 index 000000000000..ad1b1991442f --- /dev/null +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java @@ -0,0 +1,233 @@ +/* + * 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.metastore; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.ExecutionContextTestUtils; +import org.apache.hadoop.hive.metastore.HMSHandler; +import org.apache.hadoop.hive.metastore.MetaStoreTestUtils; +import org.apache.hadoop.hive.metastore.ObjectStore; +import org.apache.hadoop.hive.metastore.Warehouse; +import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest; +import org.apache.hadoop.hive.metastore.api.ColumnStatistics; +import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData; +import org.apache.hadoop.hive.metastore.api.ColumnStatisticsDesc; +import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.api.NotificationEvent; +import org.apache.hadoop.hive.metastore.api.NotificationEventRequest; +import org.apache.hadoop.hive.metastore.api.NotificationEventResponse; +import org.apache.hadoop.hive.metastore.api.Partition; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder; +import org.apache.hadoop.hive.metastore.client.builder.PartitionBuilder; +import org.apache.hadoop.hive.metastore.client.builder.TableBuilder; +import org.apache.hadoop.hive.metastore.columnstats.ColStatsBuilder; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; +import org.apache.hadoop.hive.metastore.messaging.EventMessage; +import org.apache.hadoop.hive.metastore.model.MNotificationLog; +import org.apache.hadoop.hive.metastore.model.MPartition; +import org.apache.hadoop.hive.metastore.model.MPartitionColumnStatistics; +import org.apache.hadoop.hive.metastore.model.MStorageDescriptor; +import org.apache.hadoop.hive.metastore.model.MTable; +import org.apache.hadoop.hive.metastore.utils.DirectSqlConfigurator; +import org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import javax.jdo.PersistenceManager; +import javax.jdo.Query; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME; + +/** + * Verifies batched metastore operations evict loaded JDO entities from the persistence context. + * Without eviction, long-lived RawStore instances (for example the DB notification cleaner thread) + * accumulate deleted entities in the L1 cache and can OOM. + */ +@Category(MetastoreUnitTest.class) +public class TestPersistenceContextEviction { + private static final int BATCH_SIZE = 3; + private static final int NUM_EVENTS = 12; + private static final int NUM_PARTITIONS = 12; + private static final String DB = "ec_evict_db"; + private static final String TABLE = "ec_evict_tbl"; + private static final String ENGINE = "hive"; + + private ObjectStore objectStore; + private Configuration conf; + private PersistenceManager pm; + + @Before + public void setUp() throws Exception { + conf = MetastoreConf.newMetastoreConf(); + MetastoreConf.setBoolVar(conf, ConfVars.HIVE_IN_TEST, true); + MetastoreConf.setLongVar(conf, MetastoreConf.ConfVars.EVENT_CLEAN_MAX_EVENTS, BATCH_SIZE); + MetastoreConf.setLongVar(conf, ConfVars.RAWSTORE_PARTITION_BATCH_SIZE, BATCH_SIZE); + MetaStoreTestUtils.setConfForStandloneMode(conf); + + String currentUrl = MetastoreConf.getVar(conf, ConfVars.CONNECT_URL_KEY); + currentUrl = currentUrl.replace(MetaStoreServerUtils.JUNIT_DATABASE_PREFIX, + String.format("%s_%s", MetaStoreServerUtils.JUNIT_DATABASE_PREFIX, UUID.randomUUID())); + MetastoreConf.setVar(conf, ConfVars.CONNECT_URL_KEY, currentUrl); + + objectStore = new ObjectStore(); + objectStore.setConf(conf); + HMSHandler.createDefaultCatalog(objectStore, new Warehouse(conf)); + pm = objectStore.createRawStoreBundle().getPersistentManager(); + } + + @Test + public void testExecutionContextCountsLoadedNotificationEvents() throws MetaException { + insertNotificationEvents(5, "payload"); + + objectStore.openTransaction(); + try { + Query query = pm.newQuery(MNotificationLog.class); + List events = (List) query.execute(); + pm.retrieveAll(events); + Assert.assertTrue("expected loaded events to remain in the persistence context", + ExecutionContextTestUtils.countCachedInstances(pm, MNotificationLog.class) >= 5); + } finally { + objectStore.rollbackTransaction(); + } + + Assert.assertEquals(0, ExecutionContextTestUtils.countCachedInstances(pm, MNotificationLog.class)); + } + + @Test + public void testCleanNotificationEventsEvictsCachedEntities() throws MetaException { + insertNotificationEvents(NUM_EVENTS, "x".repeat(50)); + + objectStore.openTransaction(); + try { + objectStore.cleanNotificationEvents(0); + Assert.assertEquals("batched notification cleanup retains deleted events in the L1 cache", NUM_EVENTS, + ExecutionContextTestUtils.countCachedInstances(pm, MNotificationLog.class)); + } finally { + objectStore.commitTransaction(); + } + + Assert.assertEquals("batched notification cleanup must not retain deleted events in the L1 cache", 0, + ExecutionContextTestUtils.countCachedInstances(pm, MNotificationLog.class)); + NotificationEventResponse response = objectStore.getNextNotification(new NotificationEventRequest()); + Assert.assertEquals(0, response.getEventsSize()); + } + + @Test + public void testDeletePartitionColumnStatisticsEvictsCachedStats() throws Exception { + List partNames; + try (DirectSqlConfigurator ignored = new DirectSqlConfigurator(conf, false)) { + partNames = createTableWithPartitionStats(NUM_PARTITIONS); + objectStore.openTransaction(); + try { + objectStore.deletePartitionColumnStatistics(DEFAULT_CATALOG_NAME, DB, TABLE, partNames, null, ENGINE); + Assert.assertEquals("batched partition column stats delete must not retain stats in the L1 cache", 0, + ExecutionContextTestUtils.countCachedInstances(pm, MPartitionColumnStatistics.class)); + Assert.assertEquals("batched partition update must not retain stats in the L1 cache", 0, + ExecutionContextTestUtils.countCachedInstances(pm, MPartition.class)); + } finally { + objectStore.commitTransaction(); + } + } + } + + @Test + public void testDropPartitionsEvictsCachedEntities() throws Exception { + List partNames; + try (DirectSqlConfigurator ignored = new DirectSqlConfigurator(conf, false)) { + partNames = createPartitionedTable(NUM_PARTITIONS); + objectStore.openTransaction(); + try { + objectStore.dropPartitions(DEFAULT_CATALOG_NAME, DB, TABLE, partNames); + Assert.assertEquals("batched partition drop must not retain partitions in the L1 cache", 0, + ExecutionContextTestUtils.countCachedInstances(pm, MPartition.class)); + Assert.assertEquals("batched partition drop must not retain storage descriptors in the L1 cache", 0, + ExecutionContextTestUtils.countCachedInstances(pm, MStorageDescriptor.class)); + } finally { + objectStore.commitTransaction(); + } + } + } + + private void insertNotificationEvents(int count, String message) throws MetaException { + for (int i = 0; i < count; i++) { + NotificationEvent event = new NotificationEvent(0, 0, + EventMessage.EventType.CREATE_DATABASE.toString(), message); + objectStore.addNotificationEvent(event); + } + } + + private List createPartitionedTable(int partitionCount) throws Exception { + objectStore.createDatabase(new DatabaseBuilder() + .setName(DB) + .setDescription("description") + .setLocation("locationurl") + .build(conf)); + + Table table = new TableBuilder() + .setDbName(DB) + .setTableName(TABLE) + .addCol("test_col1", "int") + .addPartCol("test_part_col", "int") + .build(conf); + objectStore.createTable(table); + + List partNames = new ArrayList<>(partitionCount); + for (int i = 0; i < partitionCount; i++) { + Partition partition = new PartitionBuilder() + .inTable(table) + .addValue("a" + i) + .build(conf); + objectStore.addPartition(partition); + partNames.add(Warehouse.makePartName(table.getPartitionKeys(), partition.getValues())); + } + return partNames; + } + + private List createTableWithPartitionStats(int partitionCount) throws Exception { + List partNames = createPartitionedTable(partitionCount); + Table table = objectStore.getTable(DEFAULT_CATALOG_NAME, DB, TABLE); + MTable mTable = objectStore.ensureGetMTable(DEFAULT_CATALOG_NAME, DB, TABLE); + for (int i = 0; i < partitionCount; i++) { + ColumnStatistics stats = new ColumnStatistics(); + ColumnStatisticsDesc desc = new ColumnStatisticsDesc(); + desc.setCatName(DEFAULT_CATALOG_NAME); + desc.setDbName(DB); + desc.setTableName(TABLE); + desc.setPartName(partNames.get(i)); + stats.setStatsDesc(desc); + stats.setEngine(ENGINE); + + ColumnStatisticsData data = new ColStatsBuilder<>(long.class) + .numNulls(1).numDVs(2).low(3L).high(4L).build(); + stats.setStatsObj(List.of(new ColumnStatisticsObj("test_part_col", "int", data))); + + objectStore.updatePartitionColumnStatistics(table, mTable, stats, + Warehouse.getPartValuesFromPartName(partNames.get(i)), null, -1); + } + return partNames; + } +} From 2f3e6eecf40d3cce05ce60a8e23230699751ef58 Mon Sep 17 00:00:00 2001 From: zdeng Date: Fri, 28 Aug 2026 09:26:46 +0800 Subject: [PATCH 2/4] minor fix --- .../metastore/impl/ColStatsStoreImpl.java | 2 +- .../metastore/impl/NotificationStoreImpl.java | 11 ++----- .../metastore/impl/TableStoreImpl.java | 32 ++++++++----------- 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/ColStatsStoreImpl.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/ColStatsStoreImpl.java index 874209c70fb6..2d47482a448f 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/ColStatsStoreImpl.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/ColStatsStoreImpl.java @@ -483,7 +483,6 @@ private List getMTableColumnStatistics(Table table, List validateTableCols(table, colNames); List result = Collections.emptyList(); - Query query = pm.newQuery(MTableColumnStatistics.class); result = Batchable.runBatched(batchSize, colNames, new Batchable() { @Override @@ -504,6 +503,7 @@ public List run(List input) params[i + 4] = input.get(i); } filter.append(")"); + Query query = pm.newQuery(MTableColumnStatistics.class); query.setFilter(filter.toString()); query.declareParameters(paramStr.toString()); List paritial = (List) query.executeWithArray(params); diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java index 555caed3b7ef..4006212dd8a0 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java @@ -342,15 +342,11 @@ public NotificationEventsCountResponse getNotificationEventsCount(NotificationEv Long result = 0L; long fromEventId = rqst.getFromEventId(); String inputDbName = rqst.getDbName(); - String catName = rqst.isSetCatName() ? rqst.getCatName() : getDefaultCatalog(conf); + String catName = rqst.isSetCatName() ? normalizeIdentifier(rqst.getCatName()) : getDefaultCatalog(conf); long toEventId; String paramSpecs; List paramVals = new ArrayList<>(); - // We store a catalog name in lower case in metastore and also use the same way everywhere in - // hive. - assert catName.equals(catName.toLowerCase()); - // Build the query to count events, part by part String queryStr = "select count(eventId) from " + MNotificationLog.class.getName(); // count fromEventId onwards events @@ -367,8 +363,7 @@ public NotificationEventsCountResponse getNotificationEventsCount(NotificationEv // counted. queryStr = queryStr + " && (dbName == inputDbName || dbName == null)"; paramSpecs = paramSpecs + ", java.lang.String inputDbName"; - // We store a database name in lower case in metastore. - paramVals.add(inputDbName.toLowerCase()); + paramVals.add(normalizeIdentifier(inputDbName)); } // catName could be NULL in case of transaction related events, which also need to be @@ -388,7 +383,7 @@ public NotificationEventsCountResponse getNotificationEventsCount(NotificationEv if (rqst.isSetTableNames() && !rqst.getTableNames().isEmpty()) { queryStr = queryStr + " && ("; for (String tableName : rqst.getTableNames()) { - paramVals.add(tableName.toLowerCase()); + paramVals.add(normalizeIdentifier(tableName)); queryStr = queryStr + "tableName == tableName" + paramVals.size() + " || "; paramSpecs = paramSpecs + ", java.lang.String tableName" + paramVals.size(); } diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java index 82900c08fa65..13c6fc8ca0e6 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java @@ -937,7 +937,7 @@ public Table alterTable(TableName tableName, Table newTable, String queryValidWr boolean isToTxn = isTxn && !TxnUtils.isTransactionalTable(oldt.getParameters()); if (!isToTxn && isTxn && areTxnStatsSupported) { // Transactional table is altered without a txn. Make sure there are no changes to the flag. - String errorMsg = verifyStatsChangeCtx(TableName.getDbTable(name, dbname), oldt.getParameters(), + String errorMsg = verifyStatsChangeCtx(TableName.getDbTable(dbname, name), oldt.getParameters(), newTable.getParameters(), newTable.getWriteId(), queryValidWriteIds, false); if (errorMsg != null) { throw new MetaException(errorMsg); @@ -1845,25 +1845,17 @@ public Partition alterPartition(TableName tableName, List part_vals, Par String dbname = normalizeIdentifier(tableName.getDb()); String name = normalizeIdentifier(tableName.getTable()); AtomicReference oldCd = new AtomicReference<>(); - Partition result = alterPartitionNoTxn(catName, dbname, name, part_vals, new_part, queryValidWriteIds, oldCd); + MTable table = this.getMTable(catName, dbname, name); + MPartition oldp = getMPartition(catName, dbname, name, part_vals, table); + Partition result = alterPartitionNoTxn(catName, dbname, name, oldp, new_part, queryValidWriteIds, oldCd, table); removeUnusedColumnDescriptor(oldCd.get()); return result; } /** * Alters an existing partition. Initiates copy of SD. Returns the old CD. - * @param part_vals Partition values (of the original partition instance) * @param newPart Partition object containing new information */ - private Partition alterPartitionNoTxn(String catName, String dbname, String name, - List part_vals, Partition newPart, String validWriteIds, AtomicReference oldCd) - throws InvalidObjectException, MetaException { - MTable table = this.getMTable(newPart.getCatName(), newPart.getDbName(), newPart.getTableName()); - MPartition oldp = getMPartition(catName, dbname, name, part_vals, table); - return alterPartitionNoTxn(catName, dbname, name, oldp, newPart, - validWriteIds, oldCd, table); - } - private Partition alterPartitionNoTxn(String catName, String dbname, String name, MPartition oldp, Partition newPart, String validWriteIds, @@ -1872,15 +1864,15 @@ private Partition alterPartitionNoTxn(String catName, String dbname, catName = normalizeIdentifier(catName); name = normalizeIdentifier(name); dbname = normalizeIdentifier(dbname); + if (oldp == null) { + throw new InvalidObjectException("partition does not exist."); + } MPartition newp = convertToMPart(newPart, table); MColumnDescriptor oldCD = null; MStorageDescriptor oldSD = oldp.getSd(); if (oldSD != null) { oldCD = oldSD.getCD(); } - if (newp == null) { - throw new InvalidObjectException("partition does not exist."); - } oldp.setValues(newp.getValues()); oldp.setPartitionName(newp.getPartitionName()); boolean isTxn = TxnUtils.isTransactionalTable(table.getParameters()); @@ -1968,7 +1960,7 @@ protected List alterPartitionsInternal(MTable table, throw new MetaException("Invalid DB name : " + tmpPart.getDbName()); } if (!tmpPart.getTableName().equalsIgnoreCase(tblName)) { - throw new MetaException("Invalid table name : " + tmpPart.getDbName()); + throw new MetaException("Invalid table name : " + tmpPart.getTableName()); } } return new GetListHelper(this, null) { @@ -2003,8 +1995,9 @@ private List alterPartitionsViaJdo(MTable table, List partNam mPartitionList = (List) query.executeWithArray(tblName, dbName, partNames, catName); pm.retrieveAll(mPartitionList); - if (mPartitionList.size() > newParts.size()) { - throw new MetaException("Expecting only one partition but more than one partitions are found."); + if (mPartitionList.size() != newParts.size()) { + throw new MetaException("Expected " + newParts.size() + " partitions but found " + + mPartitionList.size()); } Map, MPartition> mPartsMap = new HashMap(); @@ -2016,8 +2009,9 @@ private List alterPartitionsViaJdo(MTable table, List partNam AtomicReference oldCdRef = new AtomicReference<>(); for (Partition tmpPart : newParts) { oldCdRef.set(null); + MPartition mPart = mPartsMap.get(tmpPart.getValues()); Partition result = alterPartitionNoTxn(catName, dbName, tblName, - mPartsMap.get(tmpPart.getValues()), tmpPart, queryWriteIdList, oldCdRef, table); + mPart, tmpPart, queryWriteIdList, oldCdRef, table); results.add(result); if (oldCdRef.get() != null) { oldCds.add(oldCdRef.get()); From d4ed6509cfab06785f08585afcc4a79196161830 Mon Sep 17 00:00:00 2001 From: zdeng Date: Fri, 28 Aug 2026 10:33:28 +0800 Subject: [PATCH 3/4] clean --- .../TestPersistenceContextEviction.java | 109 ------------------ 1 file changed, 109 deletions(-) diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java index ad1b1991442f..d798e238c758 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/metastore/TestPersistenceContextEviction.java @@ -25,29 +25,14 @@ import org.apache.hadoop.hive.metastore.ObjectStore; import org.apache.hadoop.hive.metastore.Warehouse; import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest; -import org.apache.hadoop.hive.metastore.api.ColumnStatistics; -import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData; -import org.apache.hadoop.hive.metastore.api.ColumnStatisticsDesc; -import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj; import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.NotificationEvent; import org.apache.hadoop.hive.metastore.api.NotificationEventRequest; import org.apache.hadoop.hive.metastore.api.NotificationEventResponse; -import org.apache.hadoop.hive.metastore.api.Partition; -import org.apache.hadoop.hive.metastore.api.Table; -import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder; -import org.apache.hadoop.hive.metastore.client.builder.PartitionBuilder; -import org.apache.hadoop.hive.metastore.client.builder.TableBuilder; -import org.apache.hadoop.hive.metastore.columnstats.ColStatsBuilder; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; import org.apache.hadoop.hive.metastore.messaging.EventMessage; import org.apache.hadoop.hive.metastore.model.MNotificationLog; -import org.apache.hadoop.hive.metastore.model.MPartition; -import org.apache.hadoop.hive.metastore.model.MPartitionColumnStatistics; -import org.apache.hadoop.hive.metastore.model.MStorageDescriptor; -import org.apache.hadoop.hive.metastore.model.MTable; -import org.apache.hadoop.hive.metastore.utils.DirectSqlConfigurator; import org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils; import org.junit.Assert; import org.junit.Before; @@ -56,12 +41,9 @@ import javax.jdo.PersistenceManager; import javax.jdo.Query; -import java.util.ArrayList; import java.util.List; import java.util.UUID; -import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME; - /** * Verifies batched metastore operations evict loaded JDO entities from the persistence context. * Without eviction, long-lived RawStore instances (for example the DB notification cleaner thread) @@ -71,10 +53,6 @@ public class TestPersistenceContextEviction { private static final int BATCH_SIZE = 3; private static final int NUM_EVENTS = 12; - private static final int NUM_PARTITIONS = 12; - private static final String DB = "ec_evict_db"; - private static final String TABLE = "ec_evict_tbl"; - private static final String ENGINE = "hive"; private ObjectStore objectStore; private Configuration conf; @@ -136,42 +114,6 @@ public void testCleanNotificationEventsEvictsCachedEntities() throws MetaExcepti Assert.assertEquals(0, response.getEventsSize()); } - @Test - public void testDeletePartitionColumnStatisticsEvictsCachedStats() throws Exception { - List partNames; - try (DirectSqlConfigurator ignored = new DirectSqlConfigurator(conf, false)) { - partNames = createTableWithPartitionStats(NUM_PARTITIONS); - objectStore.openTransaction(); - try { - objectStore.deletePartitionColumnStatistics(DEFAULT_CATALOG_NAME, DB, TABLE, partNames, null, ENGINE); - Assert.assertEquals("batched partition column stats delete must not retain stats in the L1 cache", 0, - ExecutionContextTestUtils.countCachedInstances(pm, MPartitionColumnStatistics.class)); - Assert.assertEquals("batched partition update must not retain stats in the L1 cache", 0, - ExecutionContextTestUtils.countCachedInstances(pm, MPartition.class)); - } finally { - objectStore.commitTransaction(); - } - } - } - - @Test - public void testDropPartitionsEvictsCachedEntities() throws Exception { - List partNames; - try (DirectSqlConfigurator ignored = new DirectSqlConfigurator(conf, false)) { - partNames = createPartitionedTable(NUM_PARTITIONS); - objectStore.openTransaction(); - try { - objectStore.dropPartitions(DEFAULT_CATALOG_NAME, DB, TABLE, partNames); - Assert.assertEquals("batched partition drop must not retain partitions in the L1 cache", 0, - ExecutionContextTestUtils.countCachedInstances(pm, MPartition.class)); - Assert.assertEquals("batched partition drop must not retain storage descriptors in the L1 cache", 0, - ExecutionContextTestUtils.countCachedInstances(pm, MStorageDescriptor.class)); - } finally { - objectStore.commitTransaction(); - } - } - } - private void insertNotificationEvents(int count, String message) throws MetaException { for (int i = 0; i < count; i++) { NotificationEvent event = new NotificationEvent(0, 0, @@ -179,55 +121,4 @@ private void insertNotificationEvents(int count, String message) throws MetaExce objectStore.addNotificationEvent(event); } } - - private List createPartitionedTable(int partitionCount) throws Exception { - objectStore.createDatabase(new DatabaseBuilder() - .setName(DB) - .setDescription("description") - .setLocation("locationurl") - .build(conf)); - - Table table = new TableBuilder() - .setDbName(DB) - .setTableName(TABLE) - .addCol("test_col1", "int") - .addPartCol("test_part_col", "int") - .build(conf); - objectStore.createTable(table); - - List partNames = new ArrayList<>(partitionCount); - for (int i = 0; i < partitionCount; i++) { - Partition partition = new PartitionBuilder() - .inTable(table) - .addValue("a" + i) - .build(conf); - objectStore.addPartition(partition); - partNames.add(Warehouse.makePartName(table.getPartitionKeys(), partition.getValues())); - } - return partNames; - } - - private List createTableWithPartitionStats(int partitionCount) throws Exception { - List partNames = createPartitionedTable(partitionCount); - Table table = objectStore.getTable(DEFAULT_CATALOG_NAME, DB, TABLE); - MTable mTable = objectStore.ensureGetMTable(DEFAULT_CATALOG_NAME, DB, TABLE); - for (int i = 0; i < partitionCount; i++) { - ColumnStatistics stats = new ColumnStatistics(); - ColumnStatisticsDesc desc = new ColumnStatisticsDesc(); - desc.setCatName(DEFAULT_CATALOG_NAME); - desc.setDbName(DB); - desc.setTableName(TABLE); - desc.setPartName(partNames.get(i)); - stats.setStatsDesc(desc); - stats.setEngine(ENGINE); - - ColumnStatisticsData data = new ColStatsBuilder<>(long.class) - .numNulls(1).numDVs(2).low(3L).high(4L).build(); - stats.setStatsObj(List.of(new ColumnStatisticsObj("test_part_col", "int", data))); - - objectStore.updatePartitionColumnStatistics(table, mTable, stats, - Warehouse.getPartValuesFromPartName(partNames.get(i)), null, -1); - } - return partNames; - } } From ce4350bf2c46ea1b1dd1608e6c7fe927031daed0 Mon Sep 17 00:00:00 2001 From: zdeng Date: Fri, 28 Aug 2026 14:57:08 +0800 Subject: [PATCH 4/4] fix - 1 --- .../hadoop/hive/metastore/metastore/impl/TableStoreImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java index 13c6fc8ca0e6..28ab0cc66190 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/TableStoreImpl.java @@ -1845,7 +1845,7 @@ public Partition alterPartition(TableName tableName, List part_vals, Par String dbname = normalizeIdentifier(tableName.getDb()); String name = normalizeIdentifier(tableName.getTable()); AtomicReference oldCd = new AtomicReference<>(); - MTable table = this.getMTable(catName, dbname, name); + MTable table = this.getMTable(new_part.getCatName(), new_part.getDbName(), new_part.getTableName()); MPartition oldp = getMPartition(catName, dbname, name, part_vals, table); Partition result = alterPartitionNoTxn(catName, dbname, name, oldp, new_part, queryValidWriteIds, oldCd, table); removeUnusedColumnDescriptor(oldCd.get());