From b6e7c8212b3b020dcaf270737cda3c0ede9e6ea3 Mon Sep 17 00:00:00 2001 From: yuqi Date: Thu, 27 Feb 2025 20:00:58 +0800 Subject: [PATCH 01/16] Add the cache mechanism for matalake and use cache to load `in-use` information --- .../gravitino/catalog/CatalogManager.java | 15 +++-- .../gravitino/metalake/MetalakeManager.java | 58 ++++++++++++++----- .../gravitino/catalog/TestCatalogManager.java | 18 +++--- .../metalake/TestMetalakeManager.java | 56 ++++++++++++++++++ 4 files changed, 120 insertions(+), 27 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 a3d55d4a72b..483434969a7 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -260,7 +260,7 @@ private ModelCatalog asModels() { private final Config config; - @VisibleForTesting final Cache catalogCache; + @VisibleForTesting static Cache catalogCache; private final EntityStore store; @@ -279,7 +279,7 @@ public CatalogManager(Config config, EntityStore store, IdGenerator idGenerator) this.idGenerator = idGenerator; long cacheEvictionIntervalInMs = config.get(Configs.CATALOG_CACHE_EVICTION_INTERVAL_MS); - this.catalogCache = + catalogCache = Caffeine.newBuilder() .expireAfterAccess(cacheEvictionIntervalInMs, TimeUnit.MILLISECONDS) .removalListener( @@ -791,12 +791,17 @@ private static boolean catalogInUse(EntityStore store, NameIdentifier ident) private static boolean getCatalogInUseValue(EntityStore store, NameIdentifier catalogIdent) { try { - CatalogEntity catalogEntity = - store.get(catalogIdent, EntityType.CATALOG, CatalogEntity.class); + CatalogWrapper wrapper = catalogCache.getIfPresent(catalogIdent); + CatalogEntity catalogEntity; + if (wrapper != null) { + catalogEntity = wrapper.catalog.entity(); + } else { + catalogEntity = store.get(catalogIdent, EntityType.CATALOG, CatalogEntity.class); + } + return (boolean) BASIC_CATALOG_PROPERTIES_METADATA.getOrDefault( catalogEntity.getProperties(), PROPERTY_IN_USE); - } catch (NoSuchEntityException e) { LOG.warn("Catalog {} does not exist", catalogIdent, e); throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG, catalogIdent); diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 0239526e5ee..e7037731467 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -20,12 +20,19 @@ import static org.apache.gravitino.Metalake.PROPERTY_IN_USE; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.Scheduler; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; +import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.io.IOException; import java.time.Instant; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; @@ -60,6 +67,21 @@ public class MetalakeManager implements MetalakeDispatcher { private final IdGenerator idGenerator; + @VisibleForTesting + static final Cache METALAKE_CACHE = + Caffeine.newBuilder() + .expireAfterAccess(24, TimeUnit.HOURS) + .removalListener((k, v, c) -> LOG.info("Closing metalake {}.", k)) + .scheduler( + Scheduler.forScheduledExecutorService( + new ScheduledThreadPoolExecutor( + 1, + new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat("metalake-cleaner-%d") + .build()))) + .build(); + /** * Constructs a MetalakeManager instance. * @@ -99,10 +121,12 @@ public static void checkMetalake(NameIdentifier ident, EntityStore store) public static boolean metalakeInUse(EntityStore store, NameIdentifier ident) throws NoSuchMetalakeException { try { - BaseMetalake metalake = store.get(ident, EntityType.METALAKE, BaseMetalake.class); + BaseMetalake metalake = METALAKE_CACHE.getIfPresent(ident); + if (metalake == null) { + metalake = store.get(ident, EntityType.METALAKE, BaseMetalake.class); + } return (boolean) metalake.propertiesMetadata().getOrDefault(metalake.properties(), PROPERTY_IN_USE); - } catch (NoSuchEntityException e) { LOG.warn("Metalake {} does not exist", ident, e); throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); @@ -141,16 +165,20 @@ public BaseMetalake[] listMetalakes() { */ @Override public BaseMetalake loadMetalake(NameIdentifier ident) throws NoSuchMetalakeException { - try { - return newMetalakeWithResolvedProperties( - store.get(ident, EntityType.METALAKE, BaseMetalake.class)); - } catch (NoSuchEntityException e) { - LOG.warn("Metalake {} does not exist", ident, e); - throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); - } catch (IOException ioe) { - LOG.error("Loading Metalake {} failed due to storage issues", ident, ioe); - throw new RuntimeException(ioe); - } + return METALAKE_CACHE.get( + ident, + k -> { + try { + return newMetalakeWithResolvedProperties( + store.get(ident, EntityType.METALAKE, BaseMetalake.class)); + } catch (NoSuchEntityException e) { + LOG.warn("Metalake {} does not exist", ident, e); + throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); + } catch (IOException ioe) { + LOG.error("Loading Metalake {} failed due to storage issues", ident, ioe); + throw new RuntimeException(ioe); + } + }); } private BaseMetalake newMetalakeWithResolvedProperties(BaseMetalake metalakeEntity) { @@ -207,6 +235,7 @@ public BaseMetalake createMetalake( try { store.put(metalake, false /* overwritten */); + METALAKE_CACHE.put(ident, metalake); return metalake; } catch (EntityAlreadyExistsException | AlreadyExistsException e) { LOG.warn("Metalake {} already exists", ident, e); @@ -235,6 +264,7 @@ public BaseMetalake alterMetalake(NameIdentifier ident, MetalakeChange... change throw new MetalakeNotInUseException( "Metalake %s is not in use, please enable it first", ident); } + METALAKE_CACHE.invalidate(ident); return store.update( ident, @@ -274,6 +304,7 @@ public boolean dropMetalake(NameIdentifier ident, boolean force) throw new MetalakeInUseException( "Metalake %s is in use, please disable it first or use force option", ident); } + METALAKE_CACHE.invalidate(ident); List catalogEntities = store.list(Namespace.of(ident.name()), CatalogEntity.class, EntityType.CATALOG); @@ -294,9 +325,9 @@ public boolean dropMetalake(NameIdentifier ident, boolean force) @Override public void enableMetalake(NameIdentifier ident) throws NoSuchMetalakeException { try { - boolean inUse = metalakeInUse(store, ident); if (!inUse) { + METALAKE_CACHE.invalidate(ident); store.update( ident, BaseMetalake.class, @@ -324,6 +355,7 @@ public void disableMetalake(NameIdentifier ident) throws NoSuchMetalakeException try { boolean inUse = metalakeInUse(store, ident); if (inUse) { + METALAKE_CACHE.invalidate(ident); store.update( ident, BaseMetalake.class, 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 d11bff572f1..009c797894d 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java @@ -247,7 +247,7 @@ public void testCreateCatalog() { testProperties(props, testCatalog.properties()); Assertions.assertEquals(Catalog.Type.RELATIONAL, testCatalog.type()); - Assertions.assertNotNull(catalogManager.catalogCache.getIfPresent(ident)); + Assertions.assertNotNull(CatalogManager.catalogCache.getIfPresent(ident)); // test before creation NameIdentifier ident2 = NameIdentifier.of("metalake1", "test1"); @@ -265,7 +265,7 @@ public void testCreateCatalog() { catalogManager.createCatalog( ident2, Catalog.Type.RELATIONAL, provider, "comment", props)); Assertions.assertTrue(exception1.getMessage().contains("Metalake metalake1 does not exist")); - Assertions.assertNull(catalogManager.catalogCache.getIfPresent(ident2)); + Assertions.assertNull(CatalogManager.catalogCache.getIfPresent(ident2)); // test before creation Assertions.assertThrows( @@ -285,7 +285,7 @@ public void testCreateCatalog() { exception2.getMessage().contains("Catalog metalake.test1 already exists")); // Test if the catalog is already cached - CatalogManager.CatalogWrapper cached = catalogManager.catalogCache.getIfPresent(ident); + CatalogManager.CatalogWrapper cached = CatalogManager.catalogCache.getIfPresent(ident); Assertions.assertNotNull(cached); // Test failed creation @@ -300,7 +300,7 @@ public void testCreateCatalog() { Assertions.assertTrue( exception3.getMessage().contains("Properties are reserved and cannot be set"), exception3.getMessage()); - Assertions.assertNull(catalogManager.catalogCache.getIfPresent(failedIdent)); + Assertions.assertNull(CatalogManager.catalogCache.getIfPresent(failedIdent)); // Test failed for the second time Throwable exception4 = Assertions.assertThrows( @@ -311,7 +311,7 @@ public void testCreateCatalog() { Assertions.assertTrue( exception4.getMessage().contains("Properties are reserved and cannot be set"), exception4.getMessage()); - Assertions.assertNull(catalogManager.catalogCache.getIfPresent(failedIdent)); + Assertions.assertNull(CatalogManager.catalogCache.getIfPresent(failedIdent)); } @Test @@ -394,7 +394,7 @@ public void testLoadCatalog() { exception.getMessage().contains("Catalog metalake.test22 does not exist")); // Load operation will cache the catalog - Assertions.assertNotNull(catalogManager.catalogCache.getIfPresent(ident)); + Assertions.assertNotNull(CatalogManager.catalogCache.getIfPresent(ident)); } @Test @@ -440,8 +440,8 @@ public void testAlterCatalog() { exception.getMessage().contains("Catalog metalake.test33 does not exist")); // Alter operation will update the cache - Assertions.assertNull(catalogManager.catalogCache.getIfPresent(ident)); - Assertions.assertNotNull(catalogManager.catalogCache.getIfPresent(ident1)); + Assertions.assertNull(CatalogManager.catalogCache.getIfPresent(ident)); + Assertions.assertNotNull(CatalogManager.catalogCache.getIfPresent(ident1)); } @Test @@ -469,7 +469,7 @@ public void testDropCatalog() { Assertions.assertFalse(dropped1); // Drop operation will update the cache - Assertions.assertNull(catalogManager.catalogCache.getIfPresent(ident)); + Assertions.assertNull(CatalogManager.catalogCache.getIfPresent(ident)); } @Test diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index cc2b9fd0d06..b4db421c445 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -18,6 +18,7 @@ */ package org.apache.gravitino.metalake; +import com.github.benmanes.caffeine.cache.Cache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; import java.io.IOException; @@ -195,6 +196,61 @@ public void testDropMetalake() { Assertions.assertFalse(dropped1, "metalake should be non-existent"); } + @Test + public void testMetalakeCache() { + NameIdentifier ident = NameIdentifier.of("test51"); + Map props = ImmutableMap.of("key1", "value1"); + BaseMetalake metalake = metalakeManager.createMetalake(ident, "comment", props); + Assertions.assertEquals("test51", metalake.name()); + Assertions.assertEquals("comment", metalake.comment()); + + Cache cache = MetalakeManager.METALAKE_CACHE; + + BaseMetalake baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + Assertions.assertEquals("test51", baseMetalake.name()); + Assertions.assertEquals("comment", baseMetalake.comment()); + + metalakeManager.disableMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + metalakeManager.dropMetalake(ident); + + metalakeManager.createMetalake(ident, "comment", props); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + + metalakeManager.disableMetalake(ident); + metalakeManager.dropMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + + metalakeManager.createMetalake(ident, "comment", props); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + metalakeManager.disableMetalake(ident); + metalakeManager.dropMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + + metalakeManager.createMetalake(ident, "comment", props); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + metalakeManager.disableMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + metalakeManager.enableMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + + metalakeManager.loadMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + metalakeManager.disableMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); + } + private void testProperties(Map expectedProps, Map testProps) { expectedProps.forEach( (k, v) -> { From 0e1c41f24f3128abe479d415b9e8cf61e2880b51 Mon Sep 17 00:00:00 2001 From: yuqi Date: Thu, 27 Feb 2025 20:43:12 +0800 Subject: [PATCH 02/16] fix ut --- .../java/org/apache/gravitino/metalake/MetalakeManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index e7037731467..bb77c2cf9b9 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -235,7 +235,7 @@ public BaseMetalake createMetalake( try { store.put(metalake, false /* overwritten */); - METALAKE_CACHE.put(ident, metalake); + METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(metalake)); return metalake; } catch (EntityAlreadyExistsException | AlreadyExistsException e) { LOG.warn("Metalake {} already exists", ident, e); From 302d241176ff000e7b2cb766275248ac91d10a6b Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 28 Feb 2025 12:07:54 +0800 Subject: [PATCH 03/16] Preload all metalakes to cache. --- .../org/apache/gravitino/metalake/MetalakeManager.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index bb77c2cf9b9..d73e837d44a 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -91,6 +91,13 @@ public class MetalakeManager implements MetalakeDispatcher { public MetalakeManager(EntityStore store, IdGenerator idGenerator) { this.store = store; this.idGenerator = idGenerator; + + // pre-load all metalakes and put them into cache, this is useful when user load schema/table + // directly without list/get metalake first. + BaseMetalake[] metalakes = listMetalakes(); + for (BaseMetalake metalake : metalakes) { + METALAKE_CACHE.put(metalake.nameIdentifier(), metalake); + } } /** From 1d181a826f47f4998ef6e198916558f73a99f17a Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 28 Feb 2025 19:58:20 +0800 Subject: [PATCH 04/16] fix ci error. --- core/src/main/java/org/apache/gravitino/GravitinoEnv.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java index 57f04a0cfbf..de46c0b1b41 100644 --- a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java +++ b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java @@ -414,6 +414,9 @@ private void initGravitinoServerComponents() { // create and initialize a random id generator this.idGenerator = new RandomIdGenerator(); + // Tree lock + this.lockManager = new LockManager(config); + // Create and initialize metalake related modules, the operation chain is: // MetalakeEventDispatcher -> MetalakeNormalizeDispatcher -> MetalakeHookDispatcher -> // MetalakeManager @@ -498,9 +501,6 @@ private void initGravitinoServerComponents() { this.auxServiceManager = new AuxiliaryServiceManager(); this.auxServiceManager.serviceInit(config); - // Tree lock - this.lockManager = new LockManager(config); - // Create and initialize Tag related modules this.tagDispatcher = new TagEventDispatcher(eventBus, new TagManager(idGenerator, entityStore)); } From 9e11e3e08fe2290bb340d556f78c4fb26069d610 Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 28 Feb 2025 21:26:40 +0800 Subject: [PATCH 05/16] fix --- .../gravitino/metalake/MetalakeManager.java | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 6ef2ce5183a..fb629345555 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -290,21 +290,23 @@ public BaseMetalake alterMetalake(NameIdentifier ident, MetalakeChange... change "Metalake %s is not in use, please enable it first", ident); } METALAKE_CACHE.invalidate(ident); - return store.update( - ident, - BaseMetalake.class, - EntityType.METALAKE, - metalake -> { - BaseMetalake.Builder builder = newMetalakeBuilder(metalake); - Map newProps = - metalake.properties() == null - ? Maps.newHashMap() - : Maps.newHashMap(metalake.properties()); - builder = updateEntity(builder, newProps, changes); - - return builder.build(); - }); - + BaseMetalake baseMetalake = + store.update( + ident, + BaseMetalake.class, + EntityType.METALAKE, + metalake -> { + BaseMetalake.Builder builder = newMetalakeBuilder(metalake); + Map newProps = + metalake.properties() == null + ? Maps.newHashMap() + : Maps.newHashMap(metalake.properties()); + builder = updateEntity(builder, newProps, changes); + + return builder.build(); + }); + METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); + return baseMetalake; } catch (NoSuchEntityException ne) { LOG.warn("Metalake {} does not exist", ident, ne); throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); @@ -370,22 +372,24 @@ public void enableMetalake(NameIdentifier ident) throws NoSuchMetalakeException boolean inUse = metalakeInUse(store, ident); if (!inUse) { METALAKE_CACHE.invalidate(ident); - store.update( - ident, - BaseMetalake.class, - EntityType.METALAKE, - metalake -> { - BaseMetalake.Builder builder = newMetalakeBuilder(metalake); - - Map newProps = - metalake.properties() == null - ? Maps.newHashMap() - : Maps.newHashMap(metalake.properties()); - newProps.put(PROPERTY_IN_USE, "true"); - builder.withProperties(newProps); - - return builder.build(); - }); + BaseMetalake baseMetalake = + store.update( + ident, + BaseMetalake.class, + EntityType.METALAKE, + metalake -> { + BaseMetalake.Builder builder = newMetalakeBuilder(metalake); + + Map newProps = + metalake.properties() == null + ? Maps.newHashMap() + : Maps.newHashMap(metalake.properties()); + newProps.put(PROPERTY_IN_USE, "true"); + builder.withProperties(newProps); + + return builder.build(); + }); + METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); } return null; From aa8aa806ff0942b886340212fdfa9d1dad04fc38 Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 28 Feb 2025 22:02:06 +0800 Subject: [PATCH 06/16] fix --- .../metalake/TestMetalakeManager.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index b4db421c445..62259ad7e73 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -18,6 +18,11 @@ */ package org.apache.gravitino.metalake; +import static org.apache.gravitino.Configs.TREE_LOCK_CLEAN_INTERVAL; +import static org.apache.gravitino.Configs.TREE_LOCK_MAX_NODE_IN_MEMORY; +import static org.apache.gravitino.Configs.TREE_LOCK_MIN_NODE_IN_MEMORY; +import static org.mockito.Mockito.doReturn; + import com.github.benmanes.caffeine.cache.Cache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -26,6 +31,7 @@ import java.util.Set; import org.apache.gravitino.Config; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.MetalakeChange; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.StringIdentifier; @@ -33,6 +39,7 @@ import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; +import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.memory.TestMemoryEntityStore; @@ -41,6 +48,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.testcontainers.shaded.org.apache.commons.lang3.reflect.FieldUtils; public class TestMetalakeManager { @@ -51,12 +60,17 @@ public class TestMetalakeManager { private static Config config; @BeforeAll - public static void setUp() { - config = new Config(false) {}; + public static void setUp() throws IllegalAccessException { + config = Mockito.mock(Config.class); + + doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); + doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); + doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); entityStore = new TestMemoryEntityStore.InMemoryEntityStore(); entityStore.initialize(config); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); metalakeManager = new MetalakeManager(entityStore, new RandomIdGenerator()); } @@ -241,7 +255,7 @@ public void testMetalakeCache() { Assertions.assertNull(baseMetalake); metalakeManager.enableMetalake(ident); baseMetalake = cache.getIfPresent(ident); - Assertions.assertNull(baseMetalake); + Assertions.assertNotNull(baseMetalake); metalakeManager.loadMetalake(ident); baseMetalake = cache.getIfPresent(ident); From 1b737128144014dae6e4ca5f2bc6b15ddcfcf34b Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 3 Mar 2025 10:12:50 +0800 Subject: [PATCH 07/16] fix ci tests error --- .../gravitino/metalake/MetalakeManager.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index fb629345555..5b1871e43c8 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -372,24 +372,22 @@ public void enableMetalake(NameIdentifier ident) throws NoSuchMetalakeException boolean inUse = metalakeInUse(store, ident); if (!inUse) { METALAKE_CACHE.invalidate(ident); - BaseMetalake baseMetalake = - store.update( - ident, - BaseMetalake.class, - EntityType.METALAKE, - metalake -> { - BaseMetalake.Builder builder = newMetalakeBuilder(metalake); - - Map newProps = - metalake.properties() == null - ? Maps.newHashMap() - : Maps.newHashMap(metalake.properties()); - newProps.put(PROPERTY_IN_USE, "true"); - builder.withProperties(newProps); - - return builder.build(); - }); - METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); + store.update( + ident, + BaseMetalake.class, + EntityType.METALAKE, + metalake -> { + BaseMetalake.Builder builder = newMetalakeBuilder(metalake); + + Map newProps = + metalake.properties() == null + ? Maps.newHashMap() + : Maps.newHashMap(metalake.properties()); + newProps.put(PROPERTY_IN_USE, "true"); + builder.withProperties(newProps); + + return builder.build(); + }); } return null; From 7a36a92a7a0a72ec0af12855b9c90f428333565a Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 3 Mar 2025 12:44:07 +0800 Subject: [PATCH 08/16] Fix test error. --- .../java/org/apache/gravitino/metalake/TestMetalakeManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index 62259ad7e73..bd313b5cfff 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -255,7 +255,7 @@ public void testMetalakeCache() { Assertions.assertNull(baseMetalake); metalakeManager.enableMetalake(ident); baseMetalake = cache.getIfPresent(ident); - Assertions.assertNotNull(baseMetalake); + Assertions.assertNull(baseMetalake); metalakeManager.loadMetalake(ident); baseMetalake = cache.getIfPresent(ident); From 8262b95329347f39486dc4e71269a4aa694cda3b Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 3 Mar 2025 14:16:40 +0800 Subject: [PATCH 09/16] Fix test error. --- .../java/org/apache/gravitino/metalake/TestMetalakeManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index bd313b5cfff..6eab36ccc4f 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -254,8 +254,6 @@ public void testMetalakeCache() { baseMetalake = cache.getIfPresent(ident); Assertions.assertNull(baseMetalake); metalakeManager.enableMetalake(ident); - baseMetalake = cache.getIfPresent(ident); - Assertions.assertNull(baseMetalake); metalakeManager.loadMetalake(ident); baseMetalake = cache.getIfPresent(ident); From d037f02facf0da5c65d1b68973420bfa30818d5e Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 4 Mar 2025 14:41:21 +0800 Subject: [PATCH 10/16] Resolve test error. --- .../java/org/apache/gravitino/catalog/CatalogManager.java | 1 - .../java/org/apache/gravitino/metalake/MetalakeManager.java | 5 +++++ .../org/apache/gravitino/integration/test/util/BaseIT.java | 5 +++++ 3 files changed, 10 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 f84a918bfbb..fb806c0dd48 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -855,7 +855,6 @@ private static boolean getCatalogInUseValue(EntityStore store, NameIdentifier ca } else { catalogEntity = store.get(catalogIdent, EntityType.CATALOG, CatalogEntity.class); } - return (boolean) BASIC_CATALOG_PROPERTIES_METADATA.getOrDefault( catalogEntity.getProperties(), PROPERTY_IN_USE); diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 5b1871e43c8..ad14c98ad36 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -86,6 +86,11 @@ public class MetalakeManager implements MetalakeDispatcher { .build()))) .build(); + @VisibleForTesting + public static void clearCache() { + METALAKE_CACHE.invalidateAll(); + } + /** * Constructs a MetalakeManager instance. * diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java index f8cbe508f87..60e26bc23c6 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java @@ -55,6 +55,7 @@ import org.apache.gravitino.integration.test.container.ContainerSuite; import org.apache.gravitino.integration.test.container.MySQLContainer; import org.apache.gravitino.integration.test.container.PostgreSQLContainer; +import org.apache.gravitino.metalake.MetalakeManager; import org.apache.gravitino.server.GravitinoServer; import org.apache.gravitino.server.ServerConfig; import org.apache.gravitino.server.web.JettyServerConfig; @@ -369,6 +370,10 @@ public void stopIntegrationTest() throws IOException, InterruptedException { client.close(); } customConfigs.clear(); + + // Clear static cache in MetalakeManager + MetalakeManager.clearCache(); + LOG.info("Tearing down Gravitino Server"); } From 3c98a5dedb94cb6da9dc89cd9bb2494b6129cb77 Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 4 Mar 2025 15:21:29 +0800 Subject: [PATCH 11/16] fix --- .../gravitino/metalake/MetalakeManager.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index ad14c98ad36..e73aea4d9ac 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -185,16 +185,15 @@ public BaseMetalake[] listMetalakes() { */ @Override public BaseMetalake loadMetalake(NameIdentifier ident) throws NoSuchMetalakeException { - return METALAKE_CACHE.get( + return TreeLockUtils.doWithTreeLock( ident, - k -> { + LockType.READ, + () -> { try { - BaseMetalake baseMetalake = - TreeLockUtils.doWithTreeLock( - ident, - LockType.READ, - () -> store.get(ident, EntityType.METALAKE, BaseMetalake.class)); - return newMetalakeWithResolvedProperties(baseMetalake); + BaseMetalake baseMetalake = store.get(ident, EntityType.METALAKE, BaseMetalake.class); + baseMetalake = newMetalakeWithResolvedProperties(baseMetalake); + METALAKE_CACHE.put(ident, baseMetalake); + return baseMetalake; } catch (NoSuchEntityException e) { LOG.warn("Metalake {} does not exist", ident, e); throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); From b1d7ab42688bb951b800e55c0c7801e66ea194db Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 5 Mar 2025 11:33:22 +0800 Subject: [PATCH 12/16] fix --- .../gravitino/metalake/MetalakeManager.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index e73aea4d9ac..3be7ab1aa4d 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -376,22 +376,24 @@ public void enableMetalake(NameIdentifier ident) throws NoSuchMetalakeException boolean inUse = metalakeInUse(store, ident); if (!inUse) { METALAKE_CACHE.invalidate(ident); - store.update( - ident, - BaseMetalake.class, - EntityType.METALAKE, - metalake -> { - BaseMetalake.Builder builder = newMetalakeBuilder(metalake); - - Map newProps = - metalake.properties() == null - ? Maps.newHashMap() - : Maps.newHashMap(metalake.properties()); - newProps.put(PROPERTY_IN_USE, "true"); - builder.withProperties(newProps); - - return builder.build(); - }); + BaseMetalake baseMetalake = + store.update( + ident, + BaseMetalake.class, + EntityType.METALAKE, + metalake -> { + BaseMetalake.Builder builder = newMetalakeBuilder(metalake); + + Map newProps = + metalake.properties() == null + ? Maps.newHashMap() + : Maps.newHashMap(metalake.properties()); + newProps.put(PROPERTY_IN_USE, "true"); + builder.withProperties(newProps); + + return builder.build(); + }); + METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); } return null; From 84f66f305bada07527ed73cc2da76377d93cb878 Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 5 Mar 2025 17:55:33 +0800 Subject: [PATCH 13/16] Resolve comments --- .../gravitino/metalake/MetalakeManager.java | 34 ++++++++++--------- .../metalake/TestMetalakeManager.java | 12 +++++-- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 3be7ab1aa4d..520bc1fda0e 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -413,22 +413,24 @@ public void disableMetalake(NameIdentifier ident) throws NoSuchMetalakeException boolean inUse = metalakeInUse(store, ident); if (inUse) { METALAKE_CACHE.invalidate(ident); - store.update( - ident, - BaseMetalake.class, - EntityType.METALAKE, - metalake -> { - BaseMetalake.Builder builder = newMetalakeBuilder(metalake); - - Map newProps = - metalake.properties() == null - ? Maps.newHashMap() - : Maps.newHashMap(metalake.properties()); - newProps.put(PROPERTY_IN_USE, "false"); - builder.withProperties(newProps); - - return builder.build(); - }); + BaseMetalake baseMetalake = + store.update( + ident, + BaseMetalake.class, + EntityType.METALAKE, + metalake -> { + BaseMetalake.Builder builder = newMetalakeBuilder(metalake); + + Map newProps = + metalake.properties() == null + ? Maps.newHashMap() + : Maps.newHashMap(metalake.properties()); + newProps.put(PROPERTY_IN_USE, "false"); + builder.withProperties(newProps); + + return builder.build(); + }); + METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); } return null; } catch (IOException e) { diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index 6eab36ccc4f..4765ddc716e 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -227,8 +227,10 @@ public void testMetalakeCache() { metalakeManager.disableMetalake(ident); baseMetalake = cache.getIfPresent(ident); - Assertions.assertNull(baseMetalake); + Assertions.assertNotNull(baseMetalake); metalakeManager.dropMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNull(baseMetalake); metalakeManager.createMetalake(ident, "comment", props); baseMetalake = cache.getIfPresent(ident); @@ -252,15 +254,19 @@ public void testMetalakeCache() { Assertions.assertNotNull(baseMetalake); metalakeManager.disableMetalake(ident); baseMetalake = cache.getIfPresent(ident); - Assertions.assertNull(baseMetalake); + Assertions.assertNotNull(baseMetalake); + Assertions.assertEquals("false", baseMetalake.properties().get("in-use")); metalakeManager.enableMetalake(ident); + baseMetalake = cache.getIfPresent(ident); + Assertions.assertNotNull(baseMetalake); + Assertions.assertEquals("true", baseMetalake.properties().get("in-use")); metalakeManager.loadMetalake(ident); baseMetalake = cache.getIfPresent(ident); Assertions.assertNotNull(baseMetalake); metalakeManager.disableMetalake(ident); baseMetalake = cache.getIfPresent(ident); - Assertions.assertNull(baseMetalake); + Assertions.assertNotNull(baseMetalake); } private void testProperties(Map expectedProps, Map testProps) { From 52e65cad5755da7543fc2d9ac41fb2c69a531bfd Mon Sep 17 00:00:00 2001 From: yuqi Date: Thu, 6 Mar 2025 09:48:21 +0800 Subject: [PATCH 14/16] Revert some changes. --- .../main/java/org/apache/gravitino/catalog/CatalogManager.java | 1 + 1 file changed, 1 insertion(+) 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 fb806c0dd48..8ff2e9f624e 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -858,6 +858,7 @@ private static boolean getCatalogInUseValue(EntityStore store, NameIdentifier ca return (boolean) BASIC_CATALOG_PROPERTIES_METADATA.getOrDefault( catalogEntity.getProperties(), PROPERTY_IN_USE); + } catch (NoSuchEntityException e) { LOG.warn("Catalog {} does not exist", catalogIdent, e); throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG, catalogIdent); From 0a78a3a31ea2416bf0537afc1e275ef75fd8d362 Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 7 Mar 2025 16:09:44 +0800 Subject: [PATCH 15/16] fix --- .../gravitino/metalake/MetalakeManager.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 520bc1fda0e..1830d644ea5 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -188,20 +188,22 @@ public BaseMetalake loadMetalake(NameIdentifier ident) throws NoSuchMetalakeExce return TreeLockUtils.doWithTreeLock( ident, LockType.READ, - () -> { - try { - BaseMetalake baseMetalake = store.get(ident, EntityType.METALAKE, BaseMetalake.class); - baseMetalake = newMetalakeWithResolvedProperties(baseMetalake); - METALAKE_CACHE.put(ident, baseMetalake); - return baseMetalake; - } catch (NoSuchEntityException e) { - LOG.warn("Metalake {} does not exist", ident, e); - throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); - } catch (IOException ioe) { - LOG.error("Loading Metalake {} failed due to storage issues", ident, ioe); - throw new RuntimeException(ioe); - } - }); + () -> + METALAKE_CACHE.get( + ident, + k -> { + try { + BaseMetalake baseMetalake = + store.get(ident, EntityType.METALAKE, BaseMetalake.class); + return newMetalakeWithResolvedProperties(baseMetalake); + } catch (NoSuchEntityException e) { + LOG.warn("Metalake {} does not exist", ident, e); + throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); + } catch (IOException ioe) { + LOG.error("Loading Metalake {} failed due to storage issues", ident, ioe); + throw new RuntimeException(ioe); + } + })); } private BaseMetalake newMetalakeWithResolvedProperties(BaseMetalake metalakeEntity) { From 15cfc062166d976ae8ce4666b74b118f95683189 Mon Sep 17 00:00:00 2001 From: yuqi Date: Sat, 8 Mar 2025 10:39:59 +0800 Subject: [PATCH 16/16] fix --- .../java/org/apache/gravitino/metalake/MetalakeManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 1830d644ea5..11fab56c46f 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -311,7 +311,8 @@ public BaseMetalake alterMetalake(NameIdentifier ident, MetalakeChange... change return builder.build(); }); - METALAKE_CACHE.put(ident, newMetalakeWithResolvedProperties(baseMetalake)); + METALAKE_CACHE.put( + baseMetalake.nameIdentifier(), newMetalakeWithResolvedProperties(baseMetalake)); return baseMetalake; } catch (NoSuchEntityException ne) { LOG.warn("Metalake {} does not exist", ident, ne);