This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 c33e7d40c9 [Cherry-pick to branch-1.3] [#12504] fix(core): Fix flaky
TestCatalogManager.testCatalogCacheRemoveListener (#12514) (#12643)
c33e7d40c9 is described below
commit c33e7d40c9b55885af8cda4a984a4d6b1c89cd41
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Aug 27 17:41:03 2026 +0800
[Cherry-pick to branch-1.3] [#12504] fix(core): Fix flaky
TestCatalogManager.testCatalogCacheRemoveListener (#12514) (#12643)
**Cherry-pick Information:**
- Original commit: 933401bda9da09fcfa1a5bf4048a92bce1ab8460
- Target branch: `branch-1.3`
- Status: ⚠️ **Has conflicts - manual resolution required**
**Do not merge** until conflict markers are resolved and the
`cherry-pick-conflict` label is removed.
Please review and resolve the conflicts before merging.
---------
Co-authored-by: Qi Yu <[email protected]>
---
.../apache/gravitino/catalog/CatalogManager.java | 6 +-
.../gravitino/catalog/TestCatalogManager.java | 64 +++++++++++++---------
2 files changed, 44 insertions(+), 26 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
index 4df78ad21b..71f90b679a 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -53,6 +53,7 @@ import java.util.Properties;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
@@ -297,7 +298,10 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
@Nullable private final CatalogChangeLogListener catalogChangeLogListener;
private final IdGenerator idGenerator;
- private final List<Consumer<NameIdentifier>> removalListeners =
Lists.newArrayList();
+
+ // Copy-on-write: listeners may be registered while the cache's removal
listener (running on a
+ // cache executor thread) is iterating this list.
+ private final List<Consumer<NameIdentifier>> removalListeners = new
CopyOnWriteArrayList<>();
private final ConcurrentHashMap<NameIdentifier, AtomicInteger>
localMutationCounts =
new ConcurrentHashMap<>();
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
index 6d2ad36bf1..2101bd062e 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -122,6 +122,9 @@ public class TestCatalogManager {
void reset() throws IOException {
((InMemoryEntityStore) entityStore).clear();
entityStore.put(metalakeEntity, true);
+ // The shared CatalogManager is created once in @BeforeAll, so its cache
would otherwise keep
+ // entries created by previously executed test methods and make tests
order-dependent.
+ catalogManager.getCatalogCache().invalidateAll();
}
@AfterAll
@@ -1220,37 +1223,48 @@ public class TestCatalogManager {
}
@Test
- public void testCatalogCacheRemoveListener() {
+ public void testCatalogCacheRemoveListener() throws IOException {
NameIdentifier ident = NameIdentifier.of(metalake, "catalog");
Map<String, String> props =
ImmutableMap.of(
PROPERTY_KEY1, "value1", PROPERTY_KEY2, "value2",
PROPERTY_KEY5_PREFIX + "1", "value3");
- // Create a catalog
- catalogManager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", props);
-
- // Load the catalog to add it to the cache
- catalogManager.loadCatalog(ident);
-
Assertions.assertNotNull(catalogManager.getCatalogCache().getIfPresent(ident));
+ // Use a dedicated CatalogManager (and entity store) instead of the shared
static one: the
+ // shared instance keeps cache entries and cache removal listeners
registered by other test
+ // methods, which would make the assertions below depend on the test
execution order.
+ EntityStore store = new InMemoryEntityStore();
+ store.initialize(config);
+ store.put(metalakeEntity, true);
- // Add a listener to track removed catalogs
- Set<NameIdentifier> removedCatalogs = Sets.newConcurrentHashSet();
- catalogManager.addCatalogCacheRemoveListener(removedCatalogs::add);
-
- // Invalidate the cache to trigger the removal listener
- catalogManager.getCatalogCache().invalidate(ident);
-
- // Wait for the async eviction to complete
- await()
- .atMost(Duration.ofSeconds(5))
- .untilAsserted(
- () -> {
- Assertions.assertTrue(
- removedCatalogs.contains(ident),
- "Listener should be notified of catalog removal");
- Assertions.assertEquals(
- 1, removedCatalogs.size(), "Only one catalog should be
removed");
- });
+ try (CatalogManager manager = new CatalogManager(config, store, new
RandomIdGenerator())) {
+ // Create a catalog
+ manager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", props);
+
+ // Load the catalog to add it to the cache
+ manager.loadCatalog(ident);
+ Assertions.assertNotNull(manager.getCatalogCache().getIfPresent(ident));
+
+ // Add a listener to track removed catalogs
+ Set<NameIdentifier> removedCatalogs = Sets.newConcurrentHashSet();
+ manager.addCatalogCacheRemoveListener(removedCatalogs::add);
+
+ // Invalidate the cache to trigger the removal listener
+ manager.getCatalogCache().invalidate(ident);
+
+ // Wait for the async eviction to complete
+ await()
+ .atMost(Duration.ofSeconds(5))
+ .untilAsserted(
+ () -> {
+ Assertions.assertTrue(
+ removedCatalogs.contains(ident),
+ "Listener should be notified of catalog removal");
+ Assertions.assertEquals(
+ 1, removedCatalogs.size(), "Only one catalog should be
removed");
+ });
+ } finally {
+ store.close();
+ }
}
private void testProperties(Map<String, String> expectedProps, Map<String,
String> testProps) {
