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 aac721418d [Cherry-pick to branch-1.3] [#11406] fix(core): reload
closed catalog cache wrapper (#11407) (#11416)
aac721418d is described below
commit aac721418da05e8edb826fc552b63c1abeeb1e0d
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Jun 4 17:50:55 2026 +0800
[Cherry-pick to branch-1.3] [#11406] fix(core): reload closed catalog cache
wrapper (#11407) (#11416)
**Cherry-pick Information:**
- Original commit: 0d7e459d6026dcdbfe32e07b26766d42cc5febf7
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Qi Yu <[email protected]>
---
.../apache/gravitino/catalog/CatalogManager.java | 13 ++++-
.../gravitino/catalog/TestCatalogManager.java | 60 ++++++++++++++++++++++
2 files changed, 72 insertions(+), 1 deletion(-)
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 203b8c5555..91bd5b08b6 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -1001,13 +1001,24 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
/**
* Loads the catalog with the specified identifier, wraps it in a
CatalogWrapper, and caches the
- * wrapper for reuse.
+ * wrapper for reuse. If the cached wrapper has already been closed (its
underlying catalog is
+ * null), the stale entry is evicted and a fresh wrapper is loaded and
cached.
*
* @param ident The identifier of the catalog to load.
* @return The wrapped CatalogWrapper containing the loaded catalog.
* @throws NoSuchCatalogException If the specified catalog does not exist.
*/
public CatalogWrapper loadCatalogAndWrap(NameIdentifier ident) throws
NoSuchCatalogException {
+ CatalogWrapper wrapper = catalogCache.get(ident,
this::loadCatalogInternal);
+ if (wrapper.catalog() != null) {
+ return wrapper;
+ }
+
+ // The cached wrapper has already been closed (catalog() == null), e.g. by
a prior
+ // dropCatalog or cache eviction. Evict the stale entry and reload a fresh
one.
+ // Use a conditional remove so we do not clobber a wrapper that another
thread may
+ // have concurrently reloaded into the cache between our initial get and
this remove.
+ catalogCache.asMap().remove(ident, wrapper);
return catalogCache.get(ident, this::loadCatalogInternal);
}
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 5d978dd3b0..4880d7f258 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -1027,6 +1027,66 @@ public class TestCatalogManager {
Assertions.assertNull(catalogManager.getCatalogCache().getIfPresent(ident));
}
+ @Test
+ void testDropCatalogReloadsClosedCachedWrapper() throws Exception {
+ NameIdentifier ident = NameIdentifier.of("metalake",
"closed_cache_drop_test");
+ Map<String, String> props =
+ ImmutableMap.of(
+ "provider",
+ "test",
+ PROPERTY_KEY1,
+ "value1",
+ PROPERTY_KEY2,
+ "value2",
+ PROPERTY_KEY5_PREFIX + "1",
+ "value3");
+
+ Catalog catalog =
+ catalogManager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", props);
+ Assertions.assertDoesNotThrow(() -> catalogManager.disableCatalog(ident));
+ CatalogEntity entity = entityStore.get(ident, EntityType.CATALOG,
CatalogEntity.class);
+ FieldUtils.writeField(catalog, "entity", entity, true);
+
+ CatalogManager.CatalogWrapper closedWrapper =
catalogManager.loadCatalogAndWrap(ident);
+ closedWrapper.close();
+ Assertions.assertSame(closedWrapper,
catalogManager.getCatalogCache().getIfPresent(ident));
+
+ boolean dropped = catalogManager.dropCatalog(ident);
+
+ Assertions.assertTrue(dropped);
+ Assertions.assertFalse(entityStore.exists(ident, EntityType.CATALOG));
+
Assertions.assertNull(catalogManager.getCatalogCache().getIfPresent(ident));
+ }
+
+ @Test
+ void testLoadCatalogAndWrapDoesNotInvalidateConcurrentlyReloadedWrapper() {
+ NameIdentifier ident = NameIdentifier.of("metalake",
"concurrent_cache_reload_test");
+
+ CatalogManager.CatalogWrapper closedWrapper =
Mockito.mock(CatalogManager.CatalogWrapper.class);
+ CatalogManager.CatalogWrapper freshWrapper =
Mockito.mock(CatalogManager.CatalogWrapper.class);
+ BaseCatalog<?> freshCatalog = Mockito.mock(BaseCatalog.class);
+ Mockito.doReturn(freshCatalog).when(freshWrapper).catalog();
+ Mockito.doAnswer(
+ invocation -> {
+ catalogManager.getCatalogCache().put(ident, freshWrapper);
+ return null;
+ })
+ .when(closedWrapper)
+ .catalog();
+
+ try {
+ catalogManager.getCatalogCache().put(ident, closedWrapper);
+
+ CatalogManager.CatalogWrapper loadedWrapper =
catalogManager.loadCatalogAndWrap(ident);
+
+ Assertions.assertSame(freshWrapper, loadedWrapper);
+ Assertions.assertSame(freshWrapper,
catalogManager.getCatalogCache().getIfPresent(ident));
+ Mockito.verify(freshWrapper, Mockito.never()).close();
+ } finally {
+ catalogManager.getCatalogCache().invalidate(ident);
+ }
+ }
+
@Test
void testAlterMutableProperties() {
NameIdentifier ident = NameIdentifier.of("metalake", "test51");
