diff --git a/core/src/jcstress/java/org/apache/gravitino/cache/TestCacheIndexCoherence.java b/core/src/jcstress/java/org/apache/gravitino/cache/TestCacheIndexCoherence.java index d74f32b22c7..edbaee90b20 100644 --- a/core/src/jcstress/java/org/apache/gravitino/cache/TestCacheIndexCoherence.java +++ b/core/src/jcstress/java/org/apache/gravitino/cache/TestCacheIndexCoherence.java @@ -29,10 +29,8 @@ import org.apache.gravitino.Entity; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.SchemaEntity; -import org.apache.gravitino.utils.NameIdentifierUtil; import org.openjdk.jcstress.annotations.Actor; import org.openjdk.jcstress.annotations.Arbiter; import org.openjdk.jcstress.annotations.Description; @@ -78,13 +76,13 @@ private static AuditInfo getTestAuditInfo() { + "atomicity or thread-safety guarantees.") @State public static class InsertSameKeyCoherenceTest { - private final RadixTree indexTree = + private final RadixTree indexTree = new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); private final NameIdentifier ident = NameIdentifier.of("metalake1", "catalog1", "schema1"); private final Entity entity = getTestSchemaEntity(123L, "schema1", Namespace.of("metalake1", "catalog1"), "ident1"); - private final EntityCacheRelationKey key = EntityCacheRelationKey.of(ident, entity.type()); + private final EntityCacheKey key = EntityCacheKey.of(ident, entity.type()); private final String keyStr = key.toString(); @Actor @@ -99,50 +97,7 @@ public void actor2() { @Arbiter public void arbiter(I_Result r) { - EntityCacheRelationKey valueForExactKey = indexTree.getValueForExactKey(keyStr); - r.r1 = - (valueForExactKey != null - && Objects.equals(valueForExactKey, key) - && indexTree.size() == 1) - ? 1 - : 0; - } - } - - @JCStressTest - @Outcome.Outcomes({ - @Outcome(id = "1", expect = Expect.ACCEPTABLE, desc = "Key inserted and visible"), - @Outcome(id = "0", expect = Expect.FORBIDDEN, desc = "Key not found") - }) - @Description( - "Tests that ConcurrentRadixTree handles concurrent put() operations for the same key with a " - + "relation type. Both threads insert the same key-value pair where the key includes a " - + "relation type (ROLE_USER_REL). The test expects the tree to maintain a single entry. A " - + "forbidden result indicates incorrect key handling or data loss under concurrency.") - @State - public static class InsertSameKeyWithRelationTypeCoherenceTest { - private final RadixTree indexTree = - new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - - private final NameIdentifier ident = NameIdentifierUtil.ofRole("metalake", "role"); - private final EntityCacheRelationKey key = - EntityCacheRelationKey.of( - ident, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - private final String keyStr = key.toString(); - - @Actor - public void actor1() { - indexTree.put(keyStr, key); - } - - @Actor - public void actor2() { - indexTree.put(keyStr, key); - } - - @Arbiter - public void arbiter(I_Result r) { - EntityCacheRelationKey valueForExactKey = indexTree.getValueForExactKey(keyStr); + EntityCacheKey valueForExactKey = indexTree.getValueForExactKey(keyStr); r.r1 = (valueForExactKey != null && Objects.equals(valueForExactKey, key) @@ -164,7 +119,7 @@ public void arbiter(I_Result r) { + "indicates data loss or broken concurrency guarantees.") @State public static class InsertMultipleKeyCoherenceTest { - private final RadixTree indexTree = + private final RadixTree indexTree = new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); private final NameIdentifier ident1 = NameIdentifier.of("metalake1", "catalog1", "schema1"); @@ -173,60 +128,8 @@ public static class InsertMultipleKeyCoherenceTest { private final NameIdentifier ident2 = NameIdentifier.of("metalake1", "catalog1", "schema2"); private final Entity entity2 = getTestSchemaEntity(456L, "schema2", Namespace.of("metalake1", "catalog1"), "ident2"); - private final EntityCacheRelationKey key1 = EntityCacheRelationKey.of(ident1, entity1.type()); - private final EntityCacheRelationKey key2 = EntityCacheRelationKey.of(ident2, entity2.type()); - private final String key1Str = key1.toString(); - private final String key2Str = key2.toString(); - - @Actor - public void actor1() { - indexTree.put(key1Str, key1); - } - - @Actor - public void actor2() { - indexTree.put(key2Str, key2); - } - - @Arbiter - public void arbiter(I_Result r) { - EntityCacheRelationKey valueForExactKey1 = indexTree.getValueForExactKey(key1Str); - EntityCacheRelationKey valueForExactKey2 = indexTree.getValueForExactKey(key2Str); - r.r1 = - (valueForExactKey1 != null - && valueForExactKey2 != null - && Objects.equals(valueForExactKey1, key1) - && Objects.equals(valueForExactKey2, key2) - && indexTree.size() == 2) - ? 1 - : 0; - } - } - - @JCStressTest - @Outcome.Outcomes({ - @Outcome(id = "1", expect = Expect.ACCEPTABLE, desc = "multiple Key inserted and visible"), - @Outcome(id = "0", expect = Expect.FORBIDDEN, desc = "some Key not found") - }) - @Description( - "Tests ConcurrentRadixTree under concurrent insertions of different keys with relation types. " - + "Thread 1 inserts a key with ROLE_USER_REL, and Thread 2 inserts a key with ROLE_GROUP_REL. " - + "Both keys share the same NameIdentifier but differ by relation type. The tree should retain " - + "both entries. A forbidden result indicates relation type is not correctly distinguished.") - @State - public static class InsertMultipleKeyWithRelationTypeCoherenceTest { - private final RadixTree indexTree = - new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - - private final NameIdentifier ident1 = NameIdentifierUtil.ofRole("metalake", "role1"); - private final NameIdentifier ident2 = NameIdentifierUtil.ofRole("metalake", "role1"); - - private final EntityCacheRelationKey key1 = - EntityCacheRelationKey.of( - ident1, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - private final EntityCacheRelationKey key2 = - EntityCacheRelationKey.of( - ident2, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_GROUP_REL); + private final EntityCacheKey key1 = EntityCacheKey.of(ident1, entity1.type()); + private final EntityCacheKey key2 = EntityCacheKey.of(ident2, entity2.type()); private final String key1Str = key1.toString(); private final String key2Str = key2.toString(); @@ -242,8 +145,8 @@ public void actor2() { @Arbiter public void arbiter(I_Result r) { - EntityCacheRelationKey valueForExactKey1 = indexTree.getValueForExactKey(key1Str); - EntityCacheRelationKey valueForExactKey2 = indexTree.getValueForExactKey(key2Str); + EntityCacheKey valueForExactKey1 = indexTree.getValueForExactKey(key1Str); + EntityCacheKey valueForExactKey2 = indexTree.getValueForExactKey(key2Str); r.r1 = (valueForExactKey1 != null && valueForExactKey2 != null @@ -292,49 +195,6 @@ public void arbiter(I_Result r) { } } - @JCStressTest - @Outcome.Outcomes({ - @Outcome(id = "1", expect = Expect.ACCEPTABLE, desc = "Key does not exist after remove."), - @Outcome( - id = "0", - expect = Expect.ACCEPTABLE_INTERESTING, - desc = "Key still exists due to put/remove race.") - }) - @Description( - "Tests the race condition between put() and remove() on a key with relation type in " - + "ConcurrentRadixTree. Thread 1 inserts a key that includes a relation type " - + "(ROLE_USER_REL), while Thread 2 concurrently removes the same key. If remove " - + "happens after put, the key is deleted, which is acceptable. If put happens after remove, " - + "the key remains, which is interesting but not forbidden.") - @State - public static class PutRemoveSameKeyWithRelationTypeCoherenceTest { - - private final RadixTree indexTree = - new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - - private final NameIdentifier ident = NameIdentifierUtil.ofRole("metalake", "role"); - private final EntityCacheRelationKey key = - EntityCacheRelationKey.of( - ident, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - private final String keyStr = key.toString(); - - @Actor - public void actorPut() { - indexTree.put(keyStr, key); - } - - @Actor - public void actorRemove() { - indexTree.remove(keyStr); - } - - @Arbiter - public void arbiter(I_Result r) { - EntityCacheRelationKey value = indexTree.getValueForExactKey(keyStr); - r.r1 = (value == null) ? 1 : 0; - } - } - @JCStressTest @Outcome.Outcomes({ @Outcome( @@ -388,68 +248,6 @@ public void arbiter(II_Result r) { } } - @JCStressTest - @Outcome.Outcomes({ - @Outcome( - id = "1, 1", - expect = Expect.ACCEPTABLE, - desc = "Both values are visible — correct and expected."), - @Outcome( - id = "1, 0", - expect = Expect.FORBIDDEN, - desc = "Only v1 is visible — inconsistent cache state."), - @Outcome( - id = "0, 1", - expect = Expect.FORBIDDEN, - desc = "Only v2 is visible — inconsistent cache state."), - @Outcome( - id = "0, 0", - expect = Expect.FORBIDDEN, - desc = "No value visible — inconsistent cache state.") - }) - @Description( - "Tests race conditions between concurrent put() operations on keys with different relation " - + "types and a prefix-based scan. Thread 1 inserts a ROLE_USER_REL key, Thread 2 inserts " - + "a ROLE_GROUP_REL key. The arbiter uses getValuesForKeysStartingWith() to verify both " - + "entries are visible. Visibility of only one is a race condition and forbidden. Missing " - + "both values indicates an inconsistent cache state and is also forbidden.") - @State - public static class PutAndPrefixScanWithRelationTypeCoherenceTest { - - private final RadixTree indexTree = - new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - - private final NameIdentifier ident1 = NameIdentifierUtil.ofRole("metalake", "role1"); - private final NameIdentifier ident2 = NameIdentifierUtil.ofRole("metalake", "role2"); - - private final EntityCacheRelationKey key1 = - EntityCacheRelationKey.of( - ident1, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - private final EntityCacheRelationKey key2 = - EntityCacheRelationKey.of( - ident2, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_GROUP_REL); - private final String key1Str = key1.toString(); - private final String key2Str = key2.toString(); - - @Actor - public void actorPut1() { - indexTree.put(key1Str, key1); - } - - @Actor - public void actorPut2() { - indexTree.put(key2Str, key2); - } - - @Arbiter - public void arbiter(II_Result r) { - ImmutableList values = - ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake")); - r.r1 = values.contains(key1) ? 1 : 0; - r.r2 = values.contains(key2) ? 1 : 0; - } - } - @JCStressTest @Outcome.Outcomes({ @Outcome(id = "1", expect = Expect.ACCEPTABLE, desc = "Get sees the value"), @@ -479,38 +277,4 @@ public void actorGet(I_Result r) { r.r1 = indexTree.getValueForExactKey(key) != null ? 1 : 0; } } - - @JCStressTest - @Outcome.Outcomes({ - @Outcome(id = "1", expect = Expect.ACCEPTABLE, desc = "Get sees the value"), - @Outcome( - id = "0", - expect = Expect.ACCEPTABLE_INTERESTING, - desc = "Get sees nothing due to timing") - }) - @Description( - "Tests race condition between put() and getValueForExactKey() for relation-typed cache keys. " - + "Thread 1 inserts a ROLE_USER_REL key into the radix tree. Thread 2 concurrently attempts to " - + "retrieve the same key. If the get observes the result of the put, it returns a non-null value. " - + "Missing value is interesting but not forbidden, depending on execution timing.") - @State - public static class PutAndGetWithRelationTypeCoherenceTest { - private final RadixTree indexTree = - new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - private final NameIdentifier ident = NameIdentifierUtil.ofRole("metalake", "role"); - private final EntityCacheRelationKey key = - EntityCacheRelationKey.of( - ident, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - private final String keyStr = key.toString(); - - @Actor - public void actorPut() { - indexTree.put(keyStr, key); - } - - @Actor - public void actorGet(I_Result r) { - r.r1 = indexTree.getValueForExactKey(keyStr) != null ? 1 : 0; - } - } } diff --git a/core/src/jcstress/java/org/apache/gravitino/cache/TestCaffeineEntityCacheCoherence.java b/core/src/jcstress/java/org/apache/gravitino/cache/TestCaffeineEntityCacheCoherence.java index df1978adc4e..783e0315e2f 100644 --- a/core/src/jcstress/java/org/apache/gravitino/cache/TestCaffeineEntityCacheCoherence.java +++ b/core/src/jcstress/java/org/apache/gravitino/cache/TestCaffeineEntityCacheCoherence.java @@ -25,21 +25,15 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.time.Instant; -import java.util.List; import java.util.Optional; import org.apache.gravitino.Config; import org.apache.gravitino.Entity; import org.apache.gravitino.Namespace; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.ColumnEntity; -import org.apache.gravitino.meta.GroupEntity; -import org.apache.gravitino.meta.RoleEntity; import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.meta.TableEntity; -import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.rel.types.Types; -import org.apache.gravitino.utils.NamespaceUtil; import org.openjdk.jcstress.annotations.Actor; import org.openjdk.jcstress.annotations.Arbiter; import org.openjdk.jcstress.annotations.Description; @@ -55,11 +49,6 @@ public class TestCaffeineEntityCacheCoherence { getTestSchemaEntity(1L, "schema1", Namespace.of("metalake1", "catalog1"), "test_schema1"); private static final TableEntity tableEntity = getTestTableEntity(3L, "table1", Namespace.of("metalake1", "catalog1", "schema1")); - private static final GroupEntity groupEntity = - getTestGroupEntity(4L, "group1", "metalake1", ImmutableList.of("role1")); - private static final UserEntity userEntity = - getTestUserEntity(5L, "user1", "metalake1", ImmutableList.of(6L)); - private static final RoleEntity roleEntity = getTestRoleEntity(6L, "role1", "metalake1"); private static SchemaEntity getTestSchemaEntity( long id, String name, Namespace namespace, String comment) { @@ -83,38 +72,6 @@ private static TableEntity getTestTableEntity(long id, String name, Namespace na .build(); } - private static RoleEntity getTestRoleEntity(long id, String name, String metalake) { - return RoleEntity.builder() - .withId(id) - .withName(name) - .withNamespace(NamespaceUtil.ofRole(metalake)) - .withAuditInfo(getTestAuditInfo()) - .withSecurableObjects(ImmutableList.of()) - .build(); - } - - private static GroupEntity getTestGroupEntity( - long id, String name, String metalake, List roles) { - return GroupEntity.builder() - .withId(id) - .withName(name) - .withNamespace(NamespaceUtil.ofGroup(metalake)) - .withAuditInfo(getTestAuditInfo()) - .withRoleNames(roles) - .build(); - } - - private static UserEntity getTestUserEntity( - long id, String name, String metalake, List roles) { - return UserEntity.builder() - .withId(id) - .withName(name) - .withNamespace(NamespaceUtil.ofUser(metalake)) - .withAuditInfo(getTestAuditInfo()) - .withRoleIds(roles) - .build(); - } - private static AuditInfo getTestAuditInfo() { return AuditInfo.builder() .withCreator("admin") @@ -338,69 +295,6 @@ public void arbiter(I_Result r) { } } - @JCStressTest - @Outcome.Outcomes({ - @Outcome( - id = "2", - expect = Expect.ACCEPTABLE, - desc = "Both put() calls succeeded; both entries are visible in the cache."), - @Outcome( - id = "1", - expect = Expect.FORBIDDEN, - desc = "Only one entry is visible; potential visibility or atomicity issue."), - @Outcome( - id = "0", - expect = Expect.FORBIDDEN, - desc = - "Neither entry is visible; indicates a serious failure in write propagation or cache logic.") - }) - @Description("Concurrent put() with different ROLE relation types; expect both visible (2).") - @State - public static class ConcurrentPutDifferentKeysWithRelationTest { - private final EntityCache cache; - - public ConcurrentPutDifferentKeysWithRelationTest() { - this.cache = new CaffeineEntityCache(new Config() {}); - } - - @Actor - public void actor1() { - cache.put( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_GROUP_REL, - ImmutableList.of(groupEntity)); - } - - @Actor - public void actor2() { - cache.put( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_USER_REL, - ImmutableList.of(userEntity)); - } - - @Arbiter - public void arbiter(I_Result r) { - int count = 0; - if (cache.contains( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_USER_REL)) { - count++; - } - if (cache.contains( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_GROUP_REL)) { - count++; - } - - r.r1 = count; - } - } - @JCStressTest @Outcome.Outcomes({ @Outcome( @@ -438,58 +332,6 @@ public void arbiter(I_Result r) { } } - @JCStressTest - @Outcome.Outcomes({ - @Outcome( - id = "1", - expect = Expect.ACCEPTABLE, - desc = "Entry is visible; concurrent put() calls succeeded."), - @Outcome( - id = "0", - expect = Expect.FORBIDDEN, - desc = "Entry is missing; indicates visibility or atomicity issue.") - }) - @Description( - "Tests concurrent put() on the same key with relation type. " - + "Entry must remain visible after concurrent writes; missing indicates a bug.") - @State - public static class ConcurrentPutSameKeyWithRelationTest { - private final EntityCache cache; - - public ConcurrentPutSameKeyWithRelationTest() { - this.cache = new CaffeineEntityCache(new Config() {}); - } - - @Actor - public void actor1() { - cache.put( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_USER_REL, - ImmutableList.of(userEntity)); - } - - @Actor - public void actor2() { - cache.put( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_USER_REL, - ImmutableList.of(userEntity)); - } - - @Arbiter - public void arbiter(I_Result r) { - r.r1 = - cache.contains( - roleEntity.nameIdentifier(), - Entity.EntityType.ROLE, - SupportsRelationOperations.Type.ROLE_USER_REL) - ? 1 - : 0; - } - } - @JCStressTest @Outcome.Outcomes({ @Outcome( diff --git a/core/src/jmh/java/org/apache/gravitino/cache/AbstractEntityBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/AbstractEntityBenchmark.java index a90ce8208c4..f97cd6ebbb8 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/AbstractEntityBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/AbstractEntityBenchmark.java @@ -20,15 +20,10 @@ package org.apache.gravitino.cache; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.concurrent.TimeUnit; import org.apache.gravitino.Config; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Level; import org.openjdk.jmh.annotations.Mode; @@ -42,21 +37,19 @@ * AbstractEntityBenchmark is a JMH-based benchmark base class designed to evaluate the performance * of the {@link EntityCache} implementation in Gravitino. * - *

This class sets up a controlled environment where a configurable number of entities and their - * associated relations are preloaded into the cache before each iteration. It supports benchmarking - * cache operations such as put, get, and relation-based access under varying data volumes. + *

This class sets up a controlled environment where a configurable number of entities are + * preloaded into the cache before each iteration. It supports benchmarking cache operations such as + * put, get, contains, and invalidate under varying data volumes. * *

Features: * *

    *
  • Supports benchmarking with different entity counts (e.g., 10, 100, 1000) *
  • Measures both throughput and average execution time using JMH modes - *
  • Preloads both simple entities and relation entities (e.g., user-role mappings) *
* *

Subclasses should implement benchmark methods to evaluate specific cache operations. * - * @param the type of entity under test, must implement {@link Entity} and {@link HasIdentifier} * @see org.apache.gravitino.cache.CaffeineEntityCache * @see org.openjdk.jmh.annotations.Benchmark * @see org.apache.gravitino.cache.BenchmarkHelper @@ -64,13 +57,12 @@ @BenchmarkMode({Mode.Throughput, Mode.AverageTime}) @OutputTimeUnit(TimeUnit.SECONDS) @State(Scope.Thread) -public abstract class AbstractEntityBenchmark { +public abstract class AbstractEntityBenchmark { @Param({"10", "100", "1000"}) public int totalCnt; protected EntityCache cache; protected List entities; - protected Map> entitiesWithRelations; protected Random random = new Random(); @Setup(Level.Iteration) @@ -78,15 +70,7 @@ public void setup() { Config config = new Config() {}; this.cache = new CaffeineEntityCache(config); this.entities = BenchmarkHelper.getEntities(totalCnt); - this.entitiesWithRelations = BenchmarkHelper.getRelationEntities(totalCnt); entities.forEach(cache::put); - entitiesWithRelations.forEach( - (roleEntity, userList) -> - cache.put( - roleEntity.nameIdentifier(), - roleEntity.type(), - SupportsRelationOperations.Type.ROLE_USER_REL, - userList)); } } diff --git a/core/src/jmh/java/org/apache/gravitino/cache/BenchmarkHelper.java b/core/src/jmh/java/org/apache/gravitino/cache/BenchmarkHelper.java index 7d80a36c476..93c65b56d32 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/BenchmarkHelper.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/BenchmarkHelper.java @@ -20,8 +20,6 @@ package org.apache.gravitino.cache; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Maps; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -30,37 +28,15 @@ import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.apache.gravitino.utils.TestUtil; public class BenchmarkHelper { - public static final int RELATION_CNT = 5; private static final Random random = new Random(); private BenchmarkHelper() { // Utility class, no constructor needed } - /** - * Generates a map of entities with their relations. - * - * @param entityCnt the count of entities to generate. - * @return a map of entities with their relations. - * @param the type of the entity. - */ - public static Map> getRelationEntities( - int entityCnt) { - Map> entitiesWithRelations = Maps.newHashMap(); - for (int i = 0; i < entityCnt; i++) { - RoleEntity testRoleEntity = TestUtil.getTestRoleEntity(); - int userCnt = random.nextInt(RELATION_CNT); - List userList = BaseEntityCache.convertEntities(getUserList(testRoleEntity, userCnt)); - entitiesWithRelations.put(testRoleEntity, userList); - } - - return entitiesWithRelations; - } - /** * Generates a list of entities with the specified count. * @@ -117,15 +93,4 @@ public static void validateEntityHasIdentifier(Entity entity) { Preconditions.checkArgument( entity instanceof HasIdentifier, "Unsupported EntityType: " + entity.type()); } - - private static List getUserList(RoleEntity roleEntity, int userCnt) { - List userList = new ArrayList<>(userCnt); - List roleIds = ImmutableList.of(roleEntity.id()); - - for (int i = 0; i < userCnt; i++) { - userList.add(TestUtil.getTestUserEntity(roleIds)); - } - - return userList; - } } diff --git a/core/src/jmh/java/org/apache/gravitino/cache/ClearEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/ClearEntityCacheBenchmark.java index 4ccaea40796..b73e4739ff1 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/ClearEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/ClearEntityCacheBenchmark.java @@ -19,9 +19,6 @@ package org.apache.gravitino.cache; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.meta.ModelEntity; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Level; import org.openjdk.jmh.annotations.Setup; @@ -36,19 +33,15 @@ *

Before each invocation, the cache is pre-populated with a predefined list of entities to * ensure the {@code clear()} method operates on a non-empty cache. * - * @param the entity type being tested, must extend {@link Entity} and implement {@link - * HasIdentifier} * @see org.apache.gravitino.cache.EntityCache * @see org.openjdk.jmh.annotations.Benchmark */ -public class ClearEntityCacheBenchmark - extends AbstractEntityBenchmark { +public class ClearEntityCacheBenchmark extends AbstractEntityBenchmark { @Setup(Level.Invocation) - @SuppressWarnings("unchecked") public void prepareCacheForClear() { cache.clear(); - entities.forEach(e -> cache.put((ModelEntity) e)); + entities.forEach(e -> cache.put(e)); } @Benchmark diff --git a/core/src/jmh/java/org/apache/gravitino/cache/ContainsEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/ContainsEntityCacheBenchmark.java index 905500d1af5..13c4ec20307 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/ContainsEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/ContainsEntityCacheBenchmark.java @@ -19,26 +19,15 @@ package org.apache.gravitino.cache; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.openjdk.jmh.annotations.Benchmark; /** * EntityCacheContainsBenchmark benchmarks the performance of the {@link EntityCache#contains} - * operation for both regular entities and relation-based entries. + * operation for individual entities. * - *

This benchmark evaluates how efficiently the cache can determine whether a given entity or a - * relationship entry exists, under different data volumes (e.g., 10, 100, 1000). - * - *

It includes two benchmark methods: - * - *

    - *
  • {@code benchmarkContains}: Checks the presence of a single {@link ModelEntity} in the - * cache. - *
  • {@code benchmarkContainsWithRelation}: Checks whether a relation entry (e.g., role-to-user - * mapping) exists for a given {@link RoleEntity} and relation type. - *
+ *

This benchmark evaluates how efficiently the cache can determine whether a given entity + * exists, under different data volumes (e.g., 10, 100, 1000). * * @see org.apache.gravitino.cache.EntityCache * @see org.openjdk.jmh.annotations.Benchmark @@ -48,19 +37,8 @@ public class ContainsEntityCacheBenchmark extends AbstractEntityBenchmark { @Benchmark public boolean benchmarkContains() { int idx = random.nextInt(entities.size()); - ModelEntity sampleEntity = (ModelEntity) entities.get(idx); + ModelEntity sampleEntity = entities.get(idx); return cache.contains(sampleEntity.nameIdentifier(), sampleEntity.type()); } - - @Benchmark - @SuppressWarnings("unchecked") - public boolean benchmarkContainsWithRelation() { - RoleEntity sampleRoleEntity = (RoleEntity) BenchmarkHelper.getRandomKey(entitiesWithRelations); - - return cache.contains( - sampleRoleEntity.nameIdentifier(), - sampleRoleEntity.type(), - SupportsRelationOperations.Type.ROLE_USER_REL); - } } diff --git a/core/src/jmh/java/org/apache/gravitino/cache/GetEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/GetEntityCacheBenchmark.java index b19a5e1ae24..abf61023351 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/GetEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/GetEntityCacheBenchmark.java @@ -19,57 +19,27 @@ package org.apache.gravitino.cache; -import java.util.List; -import java.util.Optional; import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.openjdk.jmh.annotations.Benchmark; /** * EntityCacheGetBenchmark benchmarks the performance of the {@link EntityCache#getIfPresent} - * operation for both individual entities and entity relations. + * operation for individual entities. * *

This benchmark measures the efficiency of cache lookups under varying data sizes (e.g., 10, * 100, 1000 entries), helping evaluate the speed and consistency of the cache's get operations. * - *

It includes two benchmark methods: - * - *

    - *
  • {@code benchmarkGet}: Retrieves a {@link ModelEntity} by its identifier and type. - *
  • {@code benchmarkGetWithRelations}: Retrieves a list of related entities (e.g., users under - * a role) using a relation type and a {@link RoleEntity} as the lookup key. - *
- * - * @param the type of related entity, extending {@link Entity} and implementing {@link - * HasIdentifier} * @see org.apache.gravitino.cache.EntityCache * @see org.openjdk.jmh.annotations.Benchmark */ -public class GetEntityCacheBenchmark - extends AbstractEntityBenchmark { +public class GetEntityCacheBenchmark extends AbstractEntityBenchmark { @Benchmark public Entity benchmarkGet() { int idx = random.nextInt(entities.size()); - ModelEntity sampleEntity = (ModelEntity) entities.get(idx); + ModelEntity sampleEntity = entities.get(idx); return cache.getIfPresent(sampleEntity.nameIdentifier(), sampleEntity.type()).orElse(null); } - - @Benchmark - @SuppressWarnings("unchecked") - public List benchmarkGetWithRelations() { - RoleEntity sampleRoleEntity = (RoleEntity) BenchmarkHelper.getRandomKey(entitiesWithRelations); - - Optional> relationFromCache = - cache.getIfPresent( - SupportsRelationOperations.Type.ROLE_USER_REL, - sampleRoleEntity.nameIdentifier(), - sampleRoleEntity.type()); - - return relationFromCache.orElse(null); - } } diff --git a/core/src/jmh/java/org/apache/gravitino/cache/InvalidateEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/InvalidateEntityCacheBenchmark.java index 9eb3f202a34..27dfcfe5350 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/InvalidateEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/InvalidateEntityCacheBenchmark.java @@ -21,7 +21,6 @@ import org.apache.gravitino.Entity; import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.meta.ModelEntity; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Level; import org.openjdk.jmh.annotations.Setup; @@ -49,7 +48,7 @@ public class InvalidateEntityCacheBenchmark extends AbstractEntityBenchmark { @Setup(Level.Invocation) @SuppressWarnings("unchecked") public void prepareCacheForClear() { - entities.forEach(e -> cache.put((ModelEntity) e)); + entities.forEach(e -> cache.put(e)); } @Benchmark diff --git a/core/src/jmh/java/org/apache/gravitino/cache/MeasureSizeEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/MeasureSizeEntityCacheBenchmark.java index 5643feb10e9..eeedf973651 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/MeasureSizeEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/MeasureSizeEntityCacheBenchmark.java @@ -19,22 +19,15 @@ package org.apache.gravitino.cache; -import java.util.List; -import org.apache.gravitino.Config; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.openjdk.jmh.annotations.Benchmark; /** * EntityCacheSizeBenchmark benchmarks the performance and overhead of querying the cache size via * {@link EntityCache#size()} under varying data volumes. * - *

During setup, both standard entities and relation-based entities are preloaded into the cache - * using the configured entity count. This ensures that the {@code size()} method operates on a - * fully populated cache with realistic structure and distribution. + *

During setup, entities are preloaded into the cache using the configured entity count. This + * ensures that the {@code size()} method operates on a fully populated cache with realistic + * structure and distribution. * *

The benchmark includes a single method: * @@ -43,31 +36,10 @@ * cached entries. * * - * @param the type of related entity, extending {@link Entity} and implementing {@link - * HasIdentifier} * @see org.apache.gravitino.cache.EntityCache * @see org.openjdk.jmh.annotations.Benchmark */ -public class MeasureSizeEntityCacheBenchmark - extends AbstractEntityBenchmark { - - @Override - @SuppressWarnings("unchecked") - public void setup() { - Config config = new Config() {}; - this.cache = new CaffeineEntityCache(config); - this.entities = BenchmarkHelper.getEntities(totalCnt); - this.entitiesWithRelations = BenchmarkHelper.getRelationEntities(totalCnt); - - entities.forEach(e -> cache.put((ModelEntity) e)); - entitiesWithRelations.forEach( - (roleEntity, userList) -> - cache.put( - ((RoleEntity) roleEntity).nameIdentifier(), - ((RoleEntity) roleEntity).type(), - SupportsRelationOperations.Type.ROLE_USER_REL, - (List) userList)); - } +public class MeasureSizeEntityCacheBenchmark extends AbstractEntityBenchmark { @Benchmark public long entityCacheSize() { diff --git a/core/src/jmh/java/org/apache/gravitino/cache/PutEntityCacheBenchmark.java b/core/src/jmh/java/org/apache/gravitino/cache/PutEntityCacheBenchmark.java index 1a20a085491..796f76bfe93 100644 --- a/core/src/jmh/java/org/apache/gravitino/cache/PutEntityCacheBenchmark.java +++ b/core/src/jmh/java/org/apache/gravitino/cache/PutEntityCacheBenchmark.java @@ -19,38 +19,21 @@ package org.apache.gravitino.cache; -import java.util.List; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.RoleEntity; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Level; import org.openjdk.jmh.annotations.Setup; /** - * EntityCachePutBenchmark benchmarks the performance of {@link EntityCache#put} operations for both - * standard entities and relation-based entity groups. + * EntityCachePutBenchmark benchmarks the performance of {@link EntityCache#put} operations for + * standard entities. * *

Each benchmark invocation starts with a cleared cache to ensure that measurements reflect * fresh insertions without side effects from previous state. * - *

This benchmark covers two insertion scenarios: - * - *

    - *
  • {@code benchmarkPut}: Inserts a batch of {@link ModelEntity} instances into the cache. - *
  • {@code benchmarkPutWithRelation}: Inserts relation mappings between {@link RoleEntity} and - * related entities (e.g., users), using a relation type as part of the cache key. - *
- * - * @param the type of related entity, extending {@link Entity} and implementing {@link - * HasIdentifier} * @see org.apache.gravitino.cache.EntityCache * @see org.openjdk.jmh.annotations.Benchmark */ -public class PutEntityCacheBenchmark - extends AbstractEntityBenchmark { +public class PutEntityCacheBenchmark extends AbstractEntityBenchmark { @Setup(Level.Invocation) public void prepareForCachePut() { @@ -58,20 +41,7 @@ public void prepareForCachePut() { } @Benchmark - @SuppressWarnings("unchecked") public void benchmarkPut() { - entities.forEach(e -> cache.put((ModelEntity) e)); - } - - @Benchmark - @SuppressWarnings("unchecked") - public void benchmarkPutWithRelation() { - entitiesWithRelations.forEach( - (role, relationEntities) -> - cache.put( - ((RoleEntity) role).nameIdentifier(), - ((RoleEntity) role).type(), - SupportsRelationOperations.Type.ROLE_USER_REL, - (List) relationEntities)); + entities.forEach(e -> cache.put(e)); } } diff --git a/core/src/main/java/org/apache/gravitino/cache/BaseEntityCache.java b/core/src/main/java/org/apache/gravitino/cache/BaseEntityCache.java index 4f4b34e5e72..02262a993bf 100644 --- a/core/src/main/java/org/apache/gravitino/cache/BaseEntityCache.java +++ b/core/src/main/java/org/apache/gravitino/cache/BaseEntityCache.java @@ -45,6 +45,18 @@ public BaseEntityCache(Config config) { this.cacheConfig = config; } + /** + * {@inheritDoc} + * + *

Defaults to {@link Coherence#LOCAL_PER_NODE}: an in-memory, per-node cache whose changes + * must be propagated to the other nodes. A shared implementation must override this to return + * {@link Coherence#SHARED}. + */ + @Override + public Coherence coherence() { + return Coherence.LOCAL_PER_NODE; + } + /** * Returns the {@link NameIdentifier} of the entity based on its type. * diff --git a/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java b/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java index fd785edf5a0..965b3c8128e 100644 --- a/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java +++ b/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java @@ -30,12 +30,8 @@ import com.googlecode.concurrenttrees.radix.ConcurrentRadixTree; import com.googlecode.concurrenttrees.radix.RadixTree; import com.googlecode.concurrenttrees.radix.node.concrete.DefaultCharArrayNodeFactory; -import java.util.ArrayDeque; -import java.util.ArrayList; import java.util.List; -import java.util.Objects; import java.util.Optional; -import java.util.Queue; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ExecutorService; @@ -43,21 +39,27 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; -import org.apache.commons.lang3.ArrayUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.Entity; import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.meta.GenericEntity; -import org.apache.gravitino.meta.ModelVersionEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** This class implements the {@link org.apache.gravitino.cache.EntityCache} using Caffeine */ +/** + * This class implements the {@link org.apache.gravitino.cache.EntityCache} using Caffeine. + * + *

The cache stores one entry per entity, keyed by the entity's {@code NameIdentifier} and type. + * A radix-tree prefix index over the cache keys implements cascading removal: invalidating an + * entity also drops every cached descendant entry (e.g. invalidating a catalog drops the cached + * schemas and tables under it). + * + *

Relation query results are NOT cached by this implementation; relation and list operations + * always fall back to the {@code EntityStore}. Only the self-contained metadata objects listed in + * {@link #CACHEABLE_TYPES} are cached; every other type (user/group/role, model/model version, + * function, and operational entities) is read straight from the {@code EntityStore}. + */ public class CaffeineEntityCache extends BaseEntityCache { private static final int CACHE_CLEANUP_CORE_THREADS = 1; private static final int CACHE_CLEANUP_MAX_THREADS = 1; @@ -80,33 +82,40 @@ public class CaffeineEntityCache extends BaseEntityCache { private static final Logger LOG = LoggerFactory.getLogger(CaffeineEntityCache.class.getName()); + /** + * Entity types this cache is allowed to hold. Only self-contained entities are cacheable: a stale + * copy of one is at worst cosmetically old (an old comment, property, or job status), never a + * wrong pointer, and each can be invalidated with a single one-to-one key drop. + * + *

Every other type is read straight from the store. user/group/role embed relation-derived + * data (a role's securable objects, a user's / group's role names) that a per-node cache cannot + * invalidate; model/model version and function carry a load-bearing pointer (a version URI, the + * latest version, the function implementation) that would be silently wrong if served stale. + */ + private static final Set CACHEABLE_TYPES = + Sets.immutableEnumSet( + Entity.EntityType.METALAKE, + Entity.EntityType.CATALOG, + Entity.EntityType.SCHEMA, + Entity.EntityType.TABLE, + Entity.EntityType.TOPIC, + Entity.EntityType.VIEW, + Entity.EntityType.FILESET, + Entity.EntityType.TAG, + Entity.EntityType.POLICY, + Entity.EntityType.JOB); + /** Segmented locking for better concurrency */ private final SegmentedLock segmentedLock; /** Cache data structure. */ - private final Cache> cacheData; - - /** Cache reverse index structure. */ - private ReverseIndexCache reverseIndex; + private final Cache cacheData; - /** Cache Index structure. */ - private RadixTree cacheIndex; + /** Prefix index over cache keys, used for cascading removal of descendant entries. */ + private RadixTree cacheIndex; private ScheduledExecutorService scheduler; - @VisibleForTesting - public ReverseIndexCache getReverseIndex() { - return reverseIndex; - } - - private static final Set RELATION_TYPES = - Sets.newHashSet( - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - SupportsRelationOperations.Type.ROLE_USER_REL, - SupportsRelationOperations.Type.ROLE_GROUP_REL, - SupportsRelationOperations.Type.POLICY_METADATA_OBJECT_REL, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - /** * Constructs a new {@link CaffeineEntityCache}. * @@ -115,13 +124,12 @@ public ReverseIndexCache getReverseIndex() { public CaffeineEntityCache(Config cacheConfig) { super(cacheConfig); this.cacheIndex = new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - this.reverseIndex = new ReverseIndexCache(); // Initialize segmented lock int lockSegments = cacheConfig.get(Configs.CACHE_LOCK_SEGMENTS); this.segmentedLock = new SegmentedLock(lockSegments); - Caffeine> cacheDataBuilder = newBaseBuilder(cacheConfig); + Caffeine cacheDataBuilder = newBaseBuilder(cacheConfig); cacheDataBuilder .executor(CLEANUP_EXECUTOR) @@ -151,134 +159,39 @@ public CaffeineEntityCache(Config cacheConfig) { } @VisibleForTesting - public Cache> getCacheData() { + public Cache getCacheData() { return this.cacheData; } - /** {@inheritDoc} */ - @Override - public Optional> getIfPresent( - SupportsRelationOperations.Type relType, - NameIdentifier nameIdentifier, - Entity.EntityType identType) { - checkArguments(nameIdentifier, identType, relType); - - List entitiesFromCache = - cacheData.getIfPresent(EntityCacheRelationKey.of(nameIdentifier, identType, relType)); - return Optional.ofNullable(entitiesFromCache).map(BaseEntityCache::convertEntities); - } - /** {@inheritDoc} */ @Override public Optional getIfPresent( NameIdentifier ident, Entity.EntityType type) { checkArguments(ident, type); - List entitiesFromCache = cacheData.getIfPresent(EntityCacheRelationKey.of(ident, type)); - - return Optional.ofNullable(entitiesFromCache) - .filter(l -> !l.isEmpty()) - .map(entities -> convertEntity(entities.get(0))); - } - - /** {@inheritDoc} */ - @Override - public boolean invalidate( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - checkArguments(ident, type, relType); + Entity entityFromCache = cacheData.getIfPresent(EntityCacheKey.of(ident, type)); - return segmentedLock.withLock( - EntityCacheRelationKey.of(ident, type, relType), - () -> { - invalidateEntities(ident, type, Optional.of(relType)); - return true; - }); - } - - /** {@inheritDoc} */ - @Override - public boolean invalidateRelationEntry( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - checkArguments(ident, type, relType); - EntityCacheRelationKey key = EntityCacheRelationKey.of(ident, type, relType); - return segmentedLock.withLock( - key, - () -> { - // Drop the cached relation result, its index entry, and the reverse-index bookkeeping - // for this relation key only. Do NOT cascade through the reverse index to other - // entities: the reverse index is shared (e.g. all roles bound to one metadata object), - // and a BFS cascade would evict their mappings. cacheData.invalidate is explicit so it - // bypasses the removal listener; reverseIndex.remove(key) then cleans up only this - // entry's own bookkeeping (entityToReverseIndexMap + reverseIndex references to it). - cacheData.invalidate(key); - reverseIndex.remove(key); - cacheIndex.remove(key.toString()); - return true; - }); + return Optional.ofNullable(entityFromCache).map(BaseEntityCache::convertEntity); } /** {@inheritDoc} */ @Override public boolean invalidate(NameIdentifier ident, Entity.EntityType type) { checkArguments(ident, type); + EntityCacheKey key = EntityCacheKey.of(ident, type); return segmentedLock.withLock( - EntityCacheRelationKey.of(ident, type), + key, () -> { - // Clear possible relation first, then clear the main entity cache. - // For example, if a tag has been updated, apart from invalidating the relation: - // metadata_object_to_tag_rel, we also need to invalidate the tag_to_metadata_object_rel. - // Assuming a tag "tag1" is related to a metadata object "catalog", when "catalog" is - // renamed to `catalog_new`, we need to invalidate both relations to avoid stale data. - // that is: before: tag1:TAG_METADATA_OBJECT_REL -> catalog, - // catalog:TAG_METADATA_OBJECT_REL -> tag1, after: tag1:TAG_METADATA_OBJECT_REL -> null, - // catalog:TAG_METADATA_OBJECT_REL -> null. - RELATION_TYPES.forEach( - relType -> { - List relatedEntities = - cacheData.getIfPresent(EntityCacheRelationKey.of(ident, type, relType)); - if (relatedEntities != null) { - relatedEntities.stream() - .filter(e -> StringUtils.isNotBlank(((HasIdentifier) e).name())) - .forEach( - entity -> { - NameIdentifier identifier = ((HasIdentifier) entity).nameIdentifier(); - if (entity instanceof GenericEntity) { - String metalakeName = ident.namespace().level(0); - String[] names = - ArrayUtils.addFirst( - identifier.namespace().levels(), metalakeName); - names = ArrayUtils.add(names, identifier.name()); - identifier = NameIdentifier.of(names); - } - - invalidateEntities(identifier, entity.type(), Optional.of(relType)); - }); - } - }); - - RELATION_TYPES.forEach( - relType -> { - invalidateEntities(ident, type, Optional.of(relType)); - }); - - invalidateEntities(ident, type, Optional.empty()); + invalidateHierarchy(key); return true; }); } - /** {@inheritDoc} */ - @Override - public boolean contains( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - checkArguments(ident, type, relType); - return cacheData.getIfPresent(EntityCacheRelationKey.of(ident, type, relType)) != null; - } - /** {@inheritDoc} */ @Override public boolean contains(NameIdentifier ident, Entity.EntityType type) { checkArguments(ident, type); - return cacheData.getIfPresent(EntityCacheRelationKey.of(ident, type)) != null; + return cacheData.getIfPresent(EntityCacheKey.of(ident, type)) != null; } /** {@inheritDoc} */ @@ -293,24 +206,7 @@ public void clear() { segmentedLock.withGlobalLock( () -> { cacheData.invalidateAll(); - }); - } - - /** {@inheritDoc} */ - @Override - public void put( - NameIdentifier ident, - Entity.EntityType type, - SupportsRelationOperations.Type relType, - List entities) { - checkArguments(ident, type, relType); - Preconditions.checkArgument(entities != null, "Entities cannot be null"); - EntityCacheRelationKey entityCacheKey = EntityCacheRelationKey.of(ident, type, relType); - segmentedLock.withLock( - entityCacheKey, - () -> { - syncEntitiesToCache( - entityCacheKey, entities.stream().map(e -> (Entity) e).collect(Collectors.toList())); + cacheIndex = new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); }); } @@ -319,27 +215,30 @@ public void put( public void put(E entity) { Preconditions.checkArgument(entity != null, "Entity cannot be null"); + if (!CACHEABLE_TYPES.contains(entity.type())) { + return; + } + NameIdentifier identifier = getIdentFromEntity(entity); - EntityCacheRelationKey entityCacheKey = EntityCacheRelationKey.of(identifier, entity.type()); + EntityCacheKey entityCacheKey = EntityCacheKey.of(identifier, entity.type()); segmentedLock.withLock( entityCacheKey, () -> { - invalidateOnKeyChange(entity); - syncEntitiesToCache(entityCacheKey, Lists.newArrayList(entity)); + cacheData.put(entityCacheKey, entity); + // If the entry was rejected (e.g. it exceeds the maximum weight), skip indexing it. + if (cacheData.policy().getIfPresentQuietly(entityCacheKey) != null) { + cacheIndex.put(entityCacheKey.toString(), entityCacheKey); + } }); } /** {@inheritDoc} */ @Override public void invalidateOnKeyChange(E entity) { - // Invalidate the cache if inserting the entity may affect related cache keys. - // For example, inserting a model version changes the latest version of the model, - // so the corresponding model cache entry should be invalidated. - if (Objects.requireNonNull(entity.type()) == Entity.EntityType.MODEL_VERSION) { - NameIdentifier modelIdent = ((ModelVersionEntity) entity).modelIdentifier(); - invalidate(modelIdent, Entity.EntityType.MODEL); - } + // Every cacheable entity is self-contained (see CACHEABLE_TYPES), so inserting one never + // requires invalidating a different key. Kept for the SPI contract; implementations that cache + // derived entries can override this. } /** {@inheritDoc} */ @@ -383,35 +282,28 @@ protected void invalidateExpiredItem(EntityCacheKey key) { segmentedLock.withLock( key, () -> { - reverseIndex.remove(key); cacheIndex.remove(key.toString()); }); } /** - * Syncs the entities to the cache, if entities are too big and cannot put to the cache, then it - * will be removed from the cache, and cacheIndex will not be updated. + * Removes the entry for the given key and all cached descendant entries. Descendants are found + * through the prefix index: every child identifier starts with {@code parent identifier + "."}, + * so the scan is exact for children and never matches siblings sharing a name prefix (e.g. {@code + * catalog1} vs {@code catalog10}). * - * @param key The key of the entities. - * @param newEntities The new entities to sync to the cache. + * @param key The key of the entity whose subtree should be invalidated */ - private void syncEntitiesToCache(EntityCacheRelationKey key, List newEntities) { - List existingEntities = cacheData.getIfPresent(key); - - if (existingEntities != null && key.relationType() != null) { - Set merged = Sets.newLinkedHashSet(existingEntities); - merged.addAll(newEntities); - newEntities = new ArrayList<>(merged); - } - - cacheData.put(key, newEntities); - - for (Entity entity : newEntities) { - reverseIndex.indexEntity(entity, key); - } - - if (cacheData.policy().getIfPresentQuietly(key) != null) { - cacheIndex.put(key.toString(), key); + private void invalidateHierarchy(EntityCacheKey key) { + cacheData.invalidate(key); + cacheIndex.remove(key.toString()); + + String childPrefix = key.identifier().toString() + "."; + List childKeys = + Lists.newArrayList(cacheIndex.getValuesForKeysStartingWith(childPrefix)); + for (EntityCacheKey childKey : childKeys) { + cacheData.invalidate(childKey); + cacheIndex.remove(childKey.toString()); } } @@ -446,80 +338,6 @@ private Caffeine newBaseBuilder(Config cacheConfig) { return (Caffeine) builder; } - /** - * Invalidate entities with iterative BFS algorithm. - * - * @param identifier The identifier of the entity to invalidate - */ - private boolean invalidateEntities( - NameIdentifier identifier, - Entity.EntityType type, - Optional relTypeOpt) { - Queue queue = new ArrayDeque<>(); - - EntityCacheRelationKey valueForExactKey = - cacheIndex.getValueForExactKey( - relTypeOpt.isEmpty() - ? EntityCacheRelationKey.of(identifier, type).toString() - : EntityCacheRelationKey.of(identifier, type, relTypeOpt.get()).toString()); - - if (valueForExactKey == null) { - // It means the key does not exist in the cache. However, we still need to handle some cases. - // For example, we have stored a role entity in the cache and entity to role mapping in the - // reverse index. This is: cache data: role identifier -> role entity, reverse index: - // the securable object -> role. When we update the securable object, we need to invalidate - // the role entity from the cache though the securable object is not in the cache data. - valueForExactKey = EntityCacheRelationKey.of(identifier, type, relTypeOpt.orElse(null)); - } - - // The visited set to avoid processing the same key multiple times and thus causing infinite - // loop. - Set visited = Sets.newHashSet(); - queue.offer(valueForExactKey); - while (!queue.isEmpty()) { - EntityCacheKey currentKeyToRemove = queue.poll(); - if (visited.contains(currentKeyToRemove)) { - continue; - } - visited.add(currentKeyToRemove); - - cacheData.invalidate(currentKeyToRemove); - cacheIndex.remove(currentKeyToRemove.toString()); - - // Remove related entity keys - List relatedEntityKeysToRemove = - Lists.newArrayList( - cacheIndex.getValuesForKeysStartingWith(currentKeyToRemove.identifier().toString())); - queue.addAll(relatedEntityKeysToRemove); - - // Look up from reverse index to go to next depth - List> reverseKeysToRemove = - Lists.newArrayList( - reverseIndex.getValuesForKeysStartingWith( - currentKeyToRemove.identifier().toString())); - - reverseKeysToRemove.forEach( - key -> { - // Remove from reverse index - // Convert EntityCacheRelationKey to EntityCacheKey - key.stream() - .forEach( - k -> - reverseIndex - .getValuesForKeysStartingWith(k.toString()) - .forEach(rsk -> rsk.forEach(v -> reverseIndex.remove(v)))); - }); - - reverseIndex.remove(currentKeyToRemove); - Set toAdd = - Sets.newHashSet( - reverseKeysToRemove.stream().flatMap(List::stream).collect(Collectors.toList())); - queue.addAll(toAdd); - } - - return true; - } - /** Starts the cache stats monitor. */ private void startCacheStatsMonitor() { scheduler.scheduleAtFixedRate( @@ -539,19 +357,6 @@ private void startCacheStatsMonitor() { TimeUnit.MINUTES); } - /** - * Checks the arguments for the methods. All arguments must not be null. - * - * @param ident The identifier of the entity to check - * @param type The type of the entity to check - * @param relType The relation type of the entity to check - */ - private void checkArguments( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - checkArguments(ident, type); - Preconditions.checkArgument(relType != null, "relType cannot be null"); - } - /** * Checks the arguments for the methods. All arguments must not be null. * diff --git a/core/src/main/java/org/apache/gravitino/cache/Coherence.java b/core/src/main/java/org/apache/gravitino/cache/Coherence.java new file mode 100644 index 00000000000..6ed1e869444 --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/cache/Coherence.java @@ -0,0 +1,42 @@ +/* + * 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.cache; + +/** + * Describes how an {@link EntityCache} implementation stays coherent across a multi-node Gravitino + * cluster. The write path reads this marker to decide whether a change made on one node must be + * propagated to the other nodes. + */ +public enum Coherence { + /** + * Each node keeps its own copy of the cache, so a change made on one node does not clear the + * copies held by the other nodes. To stay correct across a cluster the write path must publish + * the change (via the {@code entity_change_log}) so every other node's poller invalidates its own + * copy. The default {@code caffeine} cache is {@code LOCAL_PER_NODE}. + */ + LOCAL_PER_NODE, + + /** + * A single copy is shared by the whole cluster, so a write clears it once and every node observes + * the change immediately. There is nothing per-node to propagate. The {@code redis} cache is + * {@code SHARED}. + */ + SHARED +} diff --git a/core/src/main/java/org/apache/gravitino/cache/EntityCache.java b/core/src/main/java/org/apache/gravitino/cache/EntityCache.java index 218c5099d04..f493f8931da 100644 --- a/core/src/main/java/org/apache/gravitino/cache/EntityCache.java +++ b/core/src/main/java/org/apache/gravitino/cache/EntityCache.java @@ -20,15 +20,24 @@ package org.apache.gravitino.cache; import java.util.List; -import org.apache.gravitino.Entity; /** * {@code EntityCache} is a cache interface in Gravitino designed to accelerate metadata access for - * entities. It maintains a bidirectional mapping between an {@link Entity} and its {@code - * NameIdentifier}. The cache also supports cascading removal of entries, ensuring related - * sub-entities are cleared together. + * individual entities keyed by their {@code NameIdentifier} and type. The cache supports cascading + * removal of entries, ensuring cached sub-entities are cleared together with their parent. + * + *

Relation query results (e.g. role bindings of a metadata object) are intentionally NOT part of + * this SPI; relation and list operations always fall back to the {@code EntityStore}. */ -public interface EntityCache extends SupportsEntityStoreCache, SupportsRelationEntityCache { +public interface EntityCache extends SupportsEntityStoreCache { + /** + * Returns how this cache stays coherent across a multi-node cluster. The write path uses this to + * decide whether a local change must be propagated to the other nodes (see {@link Coherence}). + * + * @return the {@link Coherence} model of this cache + */ + Coherence coherence(); + /** * Clears all entries from the cache, including data and index, resetting it to an empty state. */ diff --git a/core/src/main/java/org/apache/gravitino/cache/EntityCacheRelationKey.java b/core/src/main/java/org/apache/gravitino/cache/EntityCacheRelationKey.java deleted file mode 100644 index eac5950d4fd..00000000000 --- a/core/src/main/java/org/apache/gravitino/cache/EntityCacheRelationKey.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * 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.cache; - -import java.util.Objects; -import org.apache.gravitino.Entity; -import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; - -/** Key for Entity cache. */ -public class EntityCacheRelationKey extends EntityCacheKey { - private final SupportsRelationOperations.Type relationType; - - /** - * Creates a new instance of {@link EntityCacheRelationKey} with the given arguments. - * - * @param ident The identifier of the entity. - * @param type The type of the entity. - * @param relationType The type of the relation, it can be null. - * @return A new instance of {@link EntityCacheRelationKey}. - */ - public static EntityCacheRelationKey of( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relationType) { - return new EntityCacheRelationKey(ident, type, relationType); - } - - /** - * Creates a new instance of {@link EntityCacheRelationKey} with the given arguments. - * - * @param ident The identifier of the entity. - * @param type The type of the entity. - * @return A new instance of {@link EntityCacheRelationKey}. - */ - public static EntityCacheRelationKey of(NameIdentifier ident, Entity.EntityType type) { - return new EntityCacheRelationKey(ident, type, null); - } - - /** - * Creates a new instance of {@link EntityCacheRelationKey} with the given parameters. - * - * @param identifier The identifier of the entity. - * @param type The type of the entity. - * @param relationType The type of the relation. - */ - private EntityCacheRelationKey( - NameIdentifier identifier, - Entity.EntityType type, - SupportsRelationOperations.Type relationType) { - super(identifier, type); - this.relationType = relationType; - } - - /** - * Returns the type of the relation. - * - * @return The type of the relation. - */ - public SupportsRelationOperations.Type relationType() { - return relationType; - } - - /** - * Compares two instances of {@link EntityCacheRelationKey} for equality. The comparison is done - * by comparing the identifier, type, and relationType of the instances. - * - * @param obj The object to compare to. - * @return {@code true} if the objects are equal, {@code false} otherwise. - */ - @Override - public boolean equals(Object obj) { - if (obj == this) return true; - if (!(obj instanceof EntityCacheRelationKey)) return false; - EntityCacheRelationKey other = (EntityCacheRelationKey) obj; - - return super.equals(obj) && Objects.equals(relationType, other.relationType); - } - - /** - * Returns a hash code for this instance. The hash code is calculated by hashing the identifier, - * type, and relationType of the instance. - * - * @return A hash code for this instance. - */ - @Override - public int hashCode() { - return Objects.hash(super.identifier(), super.entityType(), relationType); - } - - /** - * Returns a string representation of this instance. The string is formatted as - * "identifier:type:relationType". - * - * @return A string representation of this instance. - */ - @Override - public String toString() { - String stringExpr = super.identifier().toString() + ":" + super.entityType().toString(); - if (relationType != null) { - stringExpr += ":" + relationType.name(); - } - - return stringExpr; - } -} diff --git a/core/src/main/java/org/apache/gravitino/cache/EntityCacheWeigher.java b/core/src/main/java/org/apache/gravitino/cache/EntityCacheWeigher.java index f9f1212dd70..0fde7e2414e 100644 --- a/core/src/main/java/org/apache/gravitino/cache/EntityCacheWeigher.java +++ b/core/src/main/java/org/apache/gravitino/cache/EntityCacheWeigher.java @@ -23,13 +23,10 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; -import java.util.List; import java.util.Map; import lombok.NonNull; import org.apache.gravitino.Entity; import org.checkerframework.checker.index.qual.NonNegative; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A {@link Weigher} implementation that calculates the weight of an entity based on its type. In @@ -53,14 +50,13 @@ *

Note: Caffeine's W-TinyLFU algorithm considers both access frequency and weight. Frequently * accessed heavier entries may still be retained over infrequently accessed lighter entries. */ -public class EntityCacheWeigher implements Weigher> { +public class EntityCacheWeigher implements Weigher { public static final int METALAKE_WEIGHT = 0; // 0 means never evict public static final int CATALOG_WEIGHT = 0; public static final int SCHEMA_WEIGHT = 100; // Lower weight = higher retention priority public static final int OTHER_WEIGHT = 200; public static final int TAG_WEIGHT = 500; public static final int POLICY_WEIGHT = 500; - private static final Logger LOG = LoggerFactory.getLogger(EntityCacheWeigher.class.getName()); private static final EntityCacheWeigher INSTANCE = new EntityCacheWeigher(); private static final Map ENTITY_WEIGHTS = ImmutableMap.of( @@ -124,17 +120,8 @@ public static EntityCacheWeigher getInstance() { /** {@inheritDoc} */ @Override public @NonNegative int weigh( - @NonNull EntityCacheKey storeEntityCacheKey, @NonNull List entities) { - int weight = 0; - for (Entity entity : entities) { - weight += calculateWeight(entity.type()); - } - - if (weight > getMaxWeight()) { - LOG.warn("Entity group exceeds max weight: {}", weight); - } - - return weight; + @NonNull EntityCacheKey storeEntityCacheKey, @NonNull Entity entity) { + return calculateWeight(entity.type()); } private int calculateWeight(Entity.EntityType entityType) { diff --git a/core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java b/core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java index fad08c2c2b6..3c09e812e15 100644 --- a/core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java +++ b/core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java @@ -25,7 +25,6 @@ import org.apache.gravitino.Entity; import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; /** A cache implementation that does not cache anything. */ public class NoOpsCache extends BaseEntityCache { @@ -108,44 +107,4 @@ public void put(E entity) { public void invalidateOnKeyChange(E entity) { // do nothing } - - /** {@inheritDoc} */ - @Override - public Optional> getIfPresent( - SupportsRelationOperations.Type relType, - NameIdentifier nameIdentifier, - Entity.EntityType identType) { - return Optional.empty(); - } - - /** {@inheritDoc} */ - @Override - public boolean invalidate( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - return false; - } - - /** {@inheritDoc} */ - @Override - public boolean invalidateRelationEntry( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - return false; - } - - /** {@inheritDoc} */ - @Override - public boolean contains( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType) { - return false; - } - - /** {@inheritDoc} */ - @Override - public void put( - NameIdentifier ident, - Entity.EntityType type, - SupportsRelationOperations.Type relType, - List entities) { - // do nothing - } } diff --git a/core/src/main/java/org/apache/gravitino/cache/ReverseIndexCache.java b/core/src/main/java/org/apache/gravitino/cache/ReverseIndexCache.java deleted file mode 100644 index a2956dbf068..00000000000 --- a/core/src/main/java/org/apache/gravitino/cache/ReverseIndexCache.java +++ /dev/null @@ -1,183 +0,0 @@ -/* - * 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.cache; - -import com.google.common.base.Preconditions; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Multimap; -import com.googlecode.concurrenttrees.radix.ConcurrentRadixTree; -import com.googlecode.concurrenttrees.radix.RadixTree; -import com.googlecode.concurrenttrees.radix.node.concrete.DefaultCharArrayNodeFactory; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import org.apache.commons.collections4.CollectionUtils; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.meta.GenericEntity; -import org.apache.gravitino.meta.GroupEntity; -import org.apache.gravitino.meta.PolicyEntity; -import org.apache.gravitino.meta.RoleEntity; -import org.apache.gravitino.meta.TagEntity; -import org.apache.gravitino.meta.UserEntity; - -/** - * Reverse index cache for managing entity relationships. This cache uses a radix tree to - * efficiently store and retrieve relationships between entities based on their keys. - */ -public class ReverseIndexCache { - private final RadixTree> reverseIndex; - - /** Registers a reverse index processor for a specific entity class. */ - private final Multimap, ReverseIndexRule> reverseIndexRules = - HashMultimap.create(); - - /** - * Map from data entity key to a list of entity cache relation keys. This is used for reverse - * indexing. - * - *

For example, a role entity may be related to multiple securable objects, so we need to - * maintain a mapping from the role entity key to the list of securable object keys. That is - * entityToReverseIndexMap: roleEntityKey -> [securableObjectKey1, securableObjectKey2, ...] - * - *

This map is used to quickly find all the related entity cache keys when we need to - * invalidate in the reverse index if a role entity is updated. The following is an example: a - * Role a has securable objects s1 and s2, so we have the following mapping:
- * cacheData: role1 -> role entity

- * reverseIndex: s1 -> [role1], s2 -> [role1]
- * - *

This map will be:
- * role1 -> [s1, s2]
- * - *

When we update role1, we need to invalidate s1 and s2 from the reverse index via this map, - * or the data will be in the memory forever. - */ - private final Map> entityToReverseIndexMap = - Maps.newConcurrentMap(); - - public ReverseIndexCache() { - this.reverseIndex = new ConcurrentRadixTree<>(new DefaultCharArrayNodeFactory()); - - registerReverseRule(UserEntity.class, ReverseIndexRules.USER_ROLE_REVERSE_RULE); - registerReverseRule(UserEntity.class, ReverseIndexRules.USER_OWNERSHIP_REVERSE_RULE); - registerReverseRule(GroupEntity.class, ReverseIndexRules.GROUP_ROLE_REVERSE_RULE); - registerReverseRule(GroupEntity.class, ReverseIndexRules.GROUP_OWNERSHIP_REVERSE_RULE); - registerReverseRule(RoleEntity.class, ReverseIndexRules.ROLE_SECURABLE_OBJECT_REVERSE_RULE); - registerReverseRule(PolicyEntity.class, ReverseIndexRules.POLICY_SECURABLE_OBJECT_REVERSE_RULE); - registerReverseRule(TagEntity.class, ReverseIndexRules.TAG_SECURABLE_OBJECT_REVERSE_RULE); - registerReverseRule( - GenericEntity.class, ReverseIndexRules.GENERIC_METADATA_OBJECT_REVERSE_RULE); - } - - public Iterable> getValuesForKeysStartingWith(String keyPrefix) { - return reverseIndex.getValuesForKeysStartingWith(keyPrefix); - } - - public boolean remove(EntityCacheKey key) { - List relatedKeys = entityToReverseIndexMap.remove(key); - if (CollectionUtils.isNotEmpty(relatedKeys)) { - for (EntityCacheKey relatedKey : relatedKeys) { - List existingKeys = reverseIndex.getValueForExactKey(relatedKey.toString()); - if (existingKeys != null && existingKeys.contains(key)) { - List newValues = Lists.newArrayList(existingKeys); - newValues.remove(key); - if (newValues.isEmpty()) { - reverseIndex.remove(relatedKey.toString()); - } else { - reverseIndex.put(relatedKey.toString(), newValues); - } - } - } - } - - return reverseIndex.remove(key.toString()); - } - - public int size() { - return reverseIndex.size(); - } - - public void put( - NameIdentifier nameIdentifier, Entity.EntityType type, EntityCacheRelationKey key) { - EntityCacheKey entityCacheKey = EntityCacheKey.of(nameIdentifier, type); - entityToReverseIndexMap.computeIfAbsent(key, k -> Lists.newArrayList()).add(entityCacheKey); - - List existingKeys = reverseIndex.getValueForExactKey(entityCacheKey.toString()); - if (existingKeys == null) { - reverseIndex.put(entityCacheKey.toString(), List.of(key)); - } else { - if (existingKeys.contains(key)) { - return; - } - - List newValues = Lists.newArrayList(existingKeys); - newValues.add(key); - reverseIndex.put(entityCacheKey.toString(), newValues); - } - } - - public List get(NameIdentifier nameIdentifier, Entity.EntityType type) { - EntityCacheKey entityCacheKey = EntityCacheKey.of(nameIdentifier, type); - return reverseIndex.getValueForExactKey(entityCacheKey.toString()); - } - - public void put(Entity entity, EntityCacheRelationKey key) { - Preconditions.checkArgument(entity != null, "EntityCacheRelationKey cannot be null"); - - if (entity instanceof HasIdentifier) { - NameIdentifier nameIdent = ((HasIdentifier) entity).nameIdentifier(); - put(nameIdent, entity.type(), key); - } - } - - public void registerReverseRule(Class entityClass, ReverseIndexRule rule) { - reverseIndexRules.put(entityClass, rule); - } - - /** Processes an entity and updates the reverse index accordingly. */ - public void indexEntity(Entity entity, EntityCacheRelationKey key) { - Collection rules = reverseIndexRules.get(entity.getClass()); - if (!rules.isEmpty()) { - for (ReverseIndexRule rule : rules) { - rule.indexEntity(entity, key, this); - } - } - } - - /** Functional interface for processing reverse index rules. */ - @FunctionalInterface - interface ReverseIndexRule { - void indexEntity(Entity entity, EntityCacheRelationKey key, ReverseIndexCache cache); - } - - @Override - public String toString() { - Iterable keys = reverseIndex.getKeysStartingWith(""); - StringBuilder sb = new StringBuilder(); - for (CharSequence key : keys) { - sb.append(key).append(" -> ").append(reverseIndex.getValueForExactKey(key.toString())); - sb.append("\n"); - } - - return sb.toString(); - } -} diff --git a/core/src/main/java/org/apache/gravitino/cache/ReverseIndexRules.java b/core/src/main/java/org/apache/gravitino/cache/ReverseIndexRules.java deleted file mode 100644 index 822f0459fd1..00000000000 --- a/core/src/main/java/org/apache/gravitino/cache/ReverseIndexRules.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * 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.cache; - -import org.apache.commons.lang3.ArrayUtils; -import org.apache.gravitino.Entity; -import org.apache.gravitino.Entity.EntityType; -import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.Namespace; -import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.meta.GenericEntity; -import org.apache.gravitino.meta.GroupEntity; -import org.apache.gravitino.meta.PolicyEntity; -import org.apache.gravitino.meta.RoleEntity; -import org.apache.gravitino.meta.TagEntity; -import org.apache.gravitino.meta.UserEntity; -import org.apache.gravitino.utils.MetadataObjectUtil; -import org.apache.gravitino.utils.NamespaceUtil; - -/** - * Reverse index rules for different entity types. This class defines how to process reverse - * indexing for UserEntity, GroupEntity, and RoleEntity.
- * For example:
- * - UserEntity role is {metalake-name}.system.user.{user-name}:USER-{serial-number}
- * - GroupEntity role is {metalake-name}.system.group.{group-name}:GROUP-{serial-number}
- * - RoleEntity role is {metalake-name}.system.role.{role-name}:ROLE-{serial-number}
- */ -public class ReverseIndexRules { - - /** UserEntity reverse index processor */ - public static final ReverseIndexCache.ReverseIndexRule USER_ROLE_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - UserEntity userEntity = (UserEntity) entity; - if (userEntity.roleNames() != null) { - userEntity - .roleNames() - .forEach( - role -> { - Namespace ns = NamespaceUtil.ofRole(userEntity.namespace().level(0)); - NameIdentifier nameIdentifier = NameIdentifier.of(ns, role); - reverseIndexCache.put(nameIdentifier, Entity.EntityType.ROLE, key); - }); - } - }; - - public static final ReverseIndexCache.ReverseIndexRule USER_OWNERSHIP_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - UserEntity userEntity = (UserEntity) entity; - // Handle Securable Objects -> User reverse index, so the key type is User and the value - // type is securable Object. - if (key.relationType() == SupportsRelationOperations.Type.OWNER_REL) { - reverseIndexCache.put(userEntity.nameIdentifier(), EntityType.USER, key); - } - }; - - /** GroupEntity reverse index processor */ - public static final ReverseIndexCache.ReverseIndexRule GROUP_ROLE_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - GroupEntity groupEntity = (GroupEntity) entity; - if (groupEntity.roleNames() != null) { - groupEntity - .roleNames() - .forEach( - role -> { - Namespace ns = NamespaceUtil.ofRole(groupEntity.namespace().level(0)); - NameIdentifier nameIdentifier = NameIdentifier.of(ns, role); - reverseIndexCache.put(nameIdentifier, Entity.EntityType.ROLE, key); - }); - } - }; - - public static final ReverseIndexCache.ReverseIndexRule GROUP_OWNERSHIP_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - GroupEntity groupEntity = (GroupEntity) entity; - // Handle Securable Objects -> Group reverse index, so the key type is group and the value - // type is securable Object. - if (key.relationType() == SupportsRelationOperations.Type.OWNER_REL) { - reverseIndexCache.put(groupEntity.nameIdentifier(), EntityType.GROUP, key); - } - }; - - /** RoleEntity reverse index processor */ - public static final ReverseIndexCache.ReverseIndexRule ROLE_SECURABLE_OBJECT_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - RoleEntity roleEntity = (RoleEntity) entity; - if (roleEntity.securableObjects() != null) { - roleEntity - .securableObjects() - .forEach( - securableObject -> { - NameIdentifier securableObjectIdent = - MetadataObjectUtil.toEntityIdent( - roleEntity.namespace().level(0), securableObject); - EntityType entityType = MetadataObjectUtil.toEntityType(securableObject.type()); - reverseIndexCache.put(securableObjectIdent, entityType, key); - }); - } - - // When this role entity is stored as part of a relation cache entry (e.g., - // METADATA_OBJECT_ROLE_REL keyed by a schema), also add a reverse mapping from the - // role's own identifier to that relation cache key. Without this, invalidating the - // role entity (e.g., after revokePrivilegesFromRole) would not propagate to the - // schema's METADATA_OBJECT_ROLE_REL cache entry, leaving stale data visible via - // listBindingRoleNames(). - if (key.relationType() != null) { - reverseIndexCache.put(roleEntity.nameIdentifier(), EntityType.ROLE, key); - } - }; - - // Keep policies/tags to objects reverse index for metadata objects, so the key are objects and - // the values are policies/tags. - // Only processes GenericEntity objects without namespace (metadata objects from tag/policy - // queries). - // Entities with namespace (views, tables) are skipped. - public static final ReverseIndexCache.ReverseIndexRule GENERIC_METADATA_OBJECT_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - GenericEntity genericEntity = (GenericEntity) entity; - EntityType type = entity.type(); - if (genericEntity.name() != null - && (genericEntity.namespace() == null || genericEntity.namespace().isEmpty())) { - // Name contains catalog.schema.object format without metalake, so prepend it. - String[] levels = genericEntity.name().split("\\."); - String metalakeName = key.identifier().namespace().levels()[0]; - NameIdentifier objectNameIdentifier = - NameIdentifier.of(ArrayUtils.addFirst(levels, metalakeName)); - reverseIndexCache.put(objectNameIdentifier, type, key); - } - }; - - // Keep objects to policies reverse index for policy objects, so the key are policies and the - // values are objects. - public static final ReverseIndexCache.ReverseIndexRule POLICY_SECURABLE_OBJECT_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - PolicyEntity policyEntity = (PolicyEntity) entity; - NameIdentifier nameIdentifier = - NameIdentifier.of(policyEntity.namespace(), policyEntity.name()); - reverseIndexCache.put(nameIdentifier, Entity.EntityType.POLICY, key); - }; - - // Keep objects to tags reverse index for tag objects, so the key are tags and the - // values are objects. - public static final ReverseIndexCache.ReverseIndexRule TAG_SECURABLE_OBJECT_REVERSE_RULE = - (entity, key, reverseIndexCache) -> { - TagEntity tagEntity = (TagEntity) entity; - NameIdentifier nameIdentifier = NameIdentifier.of(tagEntity.namespace(), tagEntity.name()); - reverseIndexCache.put(nameIdentifier, Entity.EntityType.TAG, key); - }; -} diff --git a/core/src/main/java/org/apache/gravitino/cache/SupportsRelationEntityCache.java b/core/src/main/java/org/apache/gravitino/cache/SupportsRelationEntityCache.java deleted file mode 100644 index 51b12113b59..00000000000 --- a/core/src/main/java/org/apache/gravitino/cache/SupportsRelationEntityCache.java +++ /dev/null @@ -1,105 +0,0 @@ -/* - * 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.cache; - -import java.util.List; -import java.util.Optional; -import org.apache.gravitino.Entity; -import org.apache.gravitino.HasIdentifier; -import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; - -/** - * {@code RelationEntityCache} defines relation-specific caching behavior for entity-to-entity - * relationships. - */ -public interface SupportsRelationEntityCache { - /** - * Retrieves a list of related entities from the cache, if present. - * - * @param relType the relation type - * @param nameIdentifier the name identifier of the entity to find related entities for - * @param identType the identifier type of the related entities to find - * @return a list of related entities, or an empty list if none are found - * @param The class of the related entities - */ - Optional> getIfPresent( - SupportsRelationOperations.Type relType, - NameIdentifier nameIdentifier, - Entity.EntityType identType); - - /** - * Invalidates the cached relation for the given entity and relation type. - * - * @param ident the name identifier - * @param type the entity type - * @param relType the relation type - * @return true if the cache entry was removed - */ - boolean invalidate( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType); - - /** - * Invalidates the cached relation result for the given key and cleans up the reverse-index - * bookkeeping for that entry, without cascading through the reverse index to other entities. - * - *

Unlike {@link #invalidate(NameIdentifier, Entity.EntityType, - * SupportsRelationOperations.Type)}, this does NOT perform a BFS cascade: it leaves other - * entities' caches and the reverse-index mappings of the entities referenced by this relation - * (e.g. all roles bound to one metadata object) intact. It only drops this relation result and - * the reverse-index references that point at this relation key, so the entry is rebuilt on the - * next read. Use it when a relation result is known to be stale and a full cascade would - * incorrectly evict other entities' state. - * - * @param ident the name identifier - * @param type the entity type - * @param relType the relation type - * @return true if the cache entry was removed - */ - boolean invalidateRelationEntry( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType); - - /** - * Checks whether an entity with the given name identifier, type, and relation type is present in - * the cache. - * - * @param ident the name identifier of the entity - * @param type the type of the entity - * @param relType the relation type - * @return {@code true} if the entity is cached; {@code false} otherwise - */ - boolean contains( - NameIdentifier ident, Entity.EntityType type, SupportsRelationOperations.Type relType); - - /** - * Puts a list of related entities into the cache. - * - * @param ident The name identifier of the entity to cache the related entities for - * @param type The type of the entity to cache the related entities for - * @param relType The relation type to cache the related entities for - * @param entities The list of related entities to cache - * @param The class of the related entities - */ - void put( - NameIdentifier ident, - Entity.EntityType type, - SupportsRelationOperations.Type relType, - List entities); -} diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/EntityCacheChangeLogListener.java b/core/src/main/java/org/apache/gravitino/storage/relational/EntityCacheChangeLogListener.java new file mode 100644 index 00000000000..4ef37c7cf8d --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/storage/relational/EntityCacheChangeLogListener.java @@ -0,0 +1,107 @@ +/* + * 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.storage.relational; + +import com.google.common.base.Preconditions; +import java.util.List; +import java.util.Locale; +import org.apache.gravitino.Entity.EntityType; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.cache.EntityCache; +import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Keeps a per-node {@link EntityCache} coherent across a multi-node cluster by replaying {@code + * entity_change_log} rows written by other nodes. + * + *

Every ALTER/DROP row is replayed as a direct {@link EntityCache#invalidate(NameIdentifier, + * EntityType)} for exactly the changed entity key. Because the cache indexes its keys by identifier + * prefix, invalidating a container (for example a schema) cascades to its cached children on the + * local node through that forward prefix scan; no reverse index is involved. + * + *

This listener is registered only for a {@link + * org.apache.gravitino.cache.Coherence#LOCAL_PER_NODE} cache: a shared cache has a single + * cluster-wide copy and nothing per-node to invalidate. It is called synchronously on the + * poller thread, so it performs only fast, in-memory, idempotent invalidations. + */ +public class EntityCacheChangeLogListener implements EntityChangeLogListener { + + private static final Logger LOG = LoggerFactory.getLogger(EntityCacheChangeLogListener.class); + + private final EntityCache cache; + + /** + * Creates a listener that invalidates the given entity store cache. + * + * @param cache the per-node entity store cache to keep coherent + */ + public EntityCacheChangeLogListener(EntityCache cache) { + Preconditions.checkArgument(cache != null, "cache cannot be null"); + this.cache = cache; + } + + @Override + public void onEntityChange(List changes) { + for (EntityChangeRecord change : changes) { + try { + EntityType type = entityType(change); + NameIdentifier ident = identifier(change); + if (type == null || ident == null) { + continue; + } + + LOG.debug("Invalidating entity cache due to entity change log: {} ({})", ident, type); + cache.invalidate(ident, type); + } catch (RuntimeException e) { + LOG.warn( + "Failed to process entity change log record: fullName={}, entityType={}", + change.getFullName(), + change.getEntityType(), + e); + } + } + } + + private EntityType entityType(EntityChangeRecord change) { + if (change.getEntityType() == null) { + LOG.warn("Invalid entity type in entity change log: null"); + return null; + } + try { + return EntityType.valueOf(change.getEntityType().toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + LOG.warn("Unknown entity type in entity change log: {}", change.getEntityType()); + return null; + } + } + + private NameIdentifier identifier(EntityChangeRecord change) { + String fullName = change.getFullName(); + if (fullName == null || fullName.isEmpty()) { + LOG.warn("Invalid full name in entity change log: {}", fullName); + return null; + } + // The change log stores the entity's NameIdentifier#toString(), a dot-joined full name. Split + // it back into levels the same way the catalog cache listener does; this is exact as long as + // no name segment contains a dot. + return NameIdentifier.of(fullName.split("\\.")); + } +} diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java b/core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java index a45d4b7d965..665e517f39d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java @@ -24,12 +24,11 @@ import com.google.common.collect.ImmutableMap; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Function; +import javax.annotation.Nullable; import org.apache.commons.lang3.tuple.Pair; import org.apache.gravitino.Config; import org.apache.gravitino.Configs; @@ -42,21 +41,15 @@ import org.apache.gravitino.RelationalEntity; import org.apache.gravitino.SupportsExternalIdOperations; import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.cache.CacheFactory; import org.apache.gravitino.cache.CachedEntityIdResolver; +import org.apache.gravitino.cache.Coherence; import org.apache.gravitino.cache.EntityCache; import org.apache.gravitino.cache.EntityCacheKey; -import org.apache.gravitino.cache.EntityCacheRelationKey; import org.apache.gravitino.cache.NoOpsCache; import org.apache.gravitino.exceptions.NoSuchEntityException; -import org.apache.gravitino.meta.GroupEntity; -import org.apache.gravitino.meta.RoleEntity; -import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.storage.relational.service.EntityIdService; import org.apache.gravitino.utils.Executable; -import org.apache.gravitino.utils.MetadataObjectUtil; -import org.apache.gravitino.utils.NamespaceUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -79,6 +72,11 @@ public class RelationalEntityStore private EntityChangeLogPoller entityChangeLogPoller; private EntityCache cache; + // Non-null only for a LOCAL_PER_NODE cache, which needs cross-node invalidation. A SHARED cache + // has a single cluster-wide copy, so there is nothing per-node to invalidate and no listener is + // registered. + @Nullable private EntityCacheChangeLogListener entityCacheChangeLogListener; + @VisibleForTesting public EntityCache getCache() { return cache; @@ -108,6 +106,14 @@ public void initialize(Config config) throws RuntimeException { config.get(Configs.ENTITY_CHANGE_LOG_POLL_INTERVAL_SECS), TimeUnit.SECONDS.toMillis(config.get(Configs.ENTITY_CHANGE_LOG_RETENTION_SECS)), TimeUnit.SECONDS.toMillis(config.get(Configs.ENTITY_CHANGE_LOG_CLEANUP_INTERVAL_SECS))); + + // The coherence gate: a LOCAL_PER_NODE cache keeps its own copy per node, so changes made on + // other nodes must be replayed here through the change log. A SHARED cache (or a disabled + // NoOpsCache) has nothing per-node to invalidate, so no listener is registered. + if (cache.coherence() == Coherence.LOCAL_PER_NODE && !(cache instanceof NoOpsCache)) { + this.entityCacheChangeLogListener = new EntityCacheChangeLogListener(cache); + this.entityChangeLogPoller.registerListener(entityCacheChangeLogListener); + } this.entityChangeLogPoller.start(); } @@ -154,7 +160,6 @@ public void put(E e, boolean overwritten) throws IOException, EntityAlreadyExistsException { backend.insert(e, overwritten); cache.put(e); - invalidateAggregatedRoleRelationCache(e); } @Override @@ -163,7 +168,6 @@ public E update( throws IOException, NoSuchEntityException, EntityAlreadyExistsException { E updatedEntity = backend.update(ident, entityType, updater); cache.invalidate(ident, entityType); - invalidateAggregatedRoleRelationCache(updatedEntity); return updatedEntity; } @@ -172,7 +176,7 @@ public E get( NameIdentifier ident, Entity.EntityType entityType, Class e) throws NoSuchEntityException, IOException { return cache.withCacheLock( - EntityCacheRelationKey.of(ident, entityType), + EntityCacheKey.of(ident, entityType), () -> { Optional entityFromCache = cache.getIfPresent(ident, entityType); if (entityFromCache.isPresent()) { @@ -291,22 +295,7 @@ public SupportsRelationOperations relationOperations() { public List listEntitiesByRelation( Type relType, NameIdentifier nameIdentifier, Entity.EntityType identType, boolean allFields) throws IOException { - return cache.withCacheLock( - EntityCacheRelationKey.of(nameIdentifier, identType, relType), - () -> { - Optional> entities = cache.getIfPresent(relType, nameIdentifier, identType); - if (entities.isPresent()) { - return entities.get(); - } - - // Use allFields=true to cache complete entities - List backendEntities = - backend.listEntitiesByRelation(relType, nameIdentifier, identType, true); - - cache.put(nameIdentifier, identType, relType, backendEntities); - - return backendEntities; - }); + return backend.listEntitiesByRelation(relType, nameIdentifier, identType, allFields); } @Override @@ -316,38 +305,7 @@ public List> batchListEntitiesByRelation( if (nameIdentifiers == null || nameIdentifiers.isEmpty()) { return new ArrayList<>(); } - - List lockKeys = new ArrayList<>(); - for (NameIdentifier id : nameIdentifiers) { - lockKeys.add(EntityCacheRelationKey.of(id, identType, relType)); - } - - return cache.withMultipleKeyCacheLock( - lockKeys, - () -> { - List> result = new ArrayList<>(); - List uncachedIdentifiers = new ArrayList<>(); - - for (NameIdentifier nameIdentifier : nameIdentifiers) { - Optional>> cachedRelations = - getCachedRelations(relType, nameIdentifier, identType); - if (cachedRelations.isPresent()) { - result.addAll(cachedRelations.get()); - } else { - uncachedIdentifiers.add(nameIdentifier); - } - } - - if (!uncachedIdentifiers.isEmpty()) { - List> backendRelations = - backend.batchListEntitiesByRelation(relType, uncachedIdentifiers, identType); - result.addAll(backendRelations); - - batchPopulateRelationCache(relType, identType, uncachedIdentifiers, backendRelations); - } - - return result; - }); + return backend.batchListEntitiesByRelation(relType, nameIdentifiers, identType); } @Override @@ -357,37 +315,12 @@ public E getEntityByRelation( Entity.EntityType srcType, NameIdentifier destEntityIdent) throws IOException, NoSuchEntityException { - return cache.withCacheLock( - EntityCacheRelationKey.of(srcIdentifier, srcType, relType), - () -> { - Optional> entities = cache.getIfPresent(relType, srcIdentifier, srcType); - if (entities.isPresent()) { - return entities.get().stream() - .filter(e -> e.nameIdentifier().equals(destEntityIdent)) - .findFirst() - .orElseThrow( - () -> - new NoSuchEntityException( - "No such entity with ident: %s", destEntityIdent)); - } - - // Use allFields=true to cache complete entities - List backendEntities = - backend.listEntitiesByRelation(relType, srcIdentifier, srcType, true); - - E r = - backendEntities.stream() - .filter(e -> e.nameIdentifier().equals(destEntityIdent)) - .findFirst() - .orElseThrow( - () -> - new NoSuchEntityException( - "No such entity with ident: %s", destEntityIdent)); - - cache.put(srcIdentifier, srcType, relType, backendEntities); - - return r; - }); + List backendEntities = backend.listEntitiesByRelation(relType, srcIdentifier, srcType, true); + return backendEntities.stream() + .filter(e -> e.nameIdentifier().equals(destEntityIdent)) + .findFirst() + .orElseThrow( + () -> new NoSuchEntityException("No such entity with ident: %s", destEntityIdent)); } @Override @@ -400,8 +333,10 @@ public void insertRelation( boolean override) throws IOException { backend.insertRelation(relType, srcIdentifier, srcType, dstIdentifier, dstType, override); - cache.invalidate(srcIdentifier, srcType, relType); - cache.invalidate(dstIdentifier, dstType, relType); + // Relation results are no longer cached, but the entities on both sides may embed + // relation-derived data (e.g. a user's role names), so drop their single-entity entries. + cache.invalidate(srcIdentifier, srcType); + cache.invalidate(dstIdentifier, dstType); } @Override @@ -419,9 +354,9 @@ public void batchInsertRelations( backend.batchInsertRelations( relType, srcIdentifiers, srcType, dstIdentifier, dstType, override); for (NameIdentifier ident : srcIdentifiers) { - cache.invalidate(ident, srcType, relType); + cache.invalidate(ident, srcType); } - cache.invalidate(dstIdentifier, dstType, relType); + cache.invalidate(dstIdentifier, dstType); } @Override @@ -432,20 +367,19 @@ public List updateEntityRelations( NameIdentifier[] destEntitiesToAdd, NameIdentifier[] destEntitiesToRemove) throws IOException, NoSuchEntityException, EntityAlreadyExistsException { - - // Invalidate after the backend write, not before. Invalidating before creates a window where - // a concurrent read can repopulate the cache with stale pre-commit data. List result = backend.updateEntityRelations( relType, srcEntityIdent, srcEntityType, destEntitiesToAdd, destEntitiesToRemove); - cache.invalidate(srcEntityIdent, srcEntityType, relType); + // Invalidate after the backend write, not before: invalidating first opens a window where a + // concurrent read could repopulate the cache with stale pre-commit data. + cache.invalidate(srcEntityIdent, srcEntityType); for (NameIdentifier destToAdd : destEntitiesToAdd) { - cache.invalidate(destToAdd, srcEntityType, relType); + cache.invalidate(destToAdd, srcEntityType); } for (NameIdentifier destToRemove : destEntitiesToRemove) { - cache.invalidate(destToRemove, srcEntityType, relType); + cache.invalidate(destToRemove, srcEntityType); } return result; @@ -463,111 +397,4 @@ public void batchPut(List entities, boolea throws IOException, EntityAlreadyExistsException { backend.batchPut(entities, overwritten); } - - private Optional>> getCachedRelations( - SupportsRelationOperations.Type relType, - NameIdentifier nameIdentifier, - Entity.EntityType identType) { - Optional> entitiesOpt = cache.getIfPresent(relType, nameIdentifier, identType); - if (entitiesOpt.isPresent()) { - List> cachedRelations = new ArrayList<>(); - for (E entity : entitiesOpt.get()) { - cachedRelations.add(new RelationalEntity<>(relType, nameIdentifier, identType, entity)); - } - return Optional.of(cachedRelations); - } - return Optional.empty(); - } - - private void batchPopulateRelationCache( - SupportsRelationOperations.Type relType, - Entity.EntityType identType, - List uncachedIdentifiers, - List> backendRelations) { - Map>> relationsBySource = new HashMap<>(); - for (RelationalEntity relation : backendRelations) { - relationsBySource.computeIfAbsent(relation.source(), k -> new ArrayList<>()).add(relation); - } - - for (NameIdentifier sourceId : uncachedIdentifiers) { - List> sourceRelations = relationsBySource.get(sourceId); - List entityList = new ArrayList<>(); - if (sourceRelations != null) { - for (RelationalEntity rel : sourceRelations) { - @SuppressWarnings("unchecked") - E entity = (E) rel.targetEntity(); - entityList.add(entity); - } - } - - cache.put(sourceId, identType, relType, entityList); - } - } - - /** - * Invalidates the relation cache entries keyed by the counterpart of a role-aggregating entity - * after that entity is written, so that reverse lookups reflect the change immediately. - * - *

Three entity types aggregate role relations and are mutated through {@code store.update} / - * {@code store.put}, which only invalidate the entity itself: - * - *

    - *
  • {@link RoleEntity} via {@code securableObjects} -> {@code METADATA_OBJECT_ROLE_REL}, - * invalidated per metadata object (catalog/schema/table/...); - *
  • {@link UserEntity} via {@code roleNames} -> {@code ROLE_USER_REL}, invalidated per role; - *
  • {@link GroupEntity} via {@code roleNames} -> {@code ROLE_GROUP_REL}, invalidated per - * role. - *
- * - *

The role-side BFS invalidation ({@code invalidate(roleIdent, ROLE)}) only reaches a - * counterpart's relation entry when the entity had previously been cached against it; a freshly - * granted binding was never cached there, so without this explicit invalidation the stale - * relation result is served until the entry's TTL elapses. Each entry is dropped via {@link - * EntityCache#invalidateRelationEntry} (no BFS cascade), preserving other entities' mappings. - */ - private void invalidateAggregatedRoleRelationCache(Entity entity) { - if (entity instanceof RoleEntity) { - RoleEntity roleEntity = (RoleEntity) entity; - List securableObjects = roleEntity.securableObjects(); - if (securableObjects == null || securableObjects.isEmpty()) { - return; - } - String metalake = roleEntity.namespace().level(0); - for (SecurableObject securableObject : securableObjects) { - cache.invalidateRelationEntry( - MetadataObjectUtil.toEntityIdent(metalake, securableObject), - MetadataObjectUtil.toEntityType(securableObject.type()), - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL); - } - } else if (entity instanceof UserEntity) { - UserEntity userEntity = (UserEntity) entity; - invalidateRoleGranteeRelations( - userEntity.namespace().level(0), - userEntity.roleNames(), - SupportsRelationOperations.Type.ROLE_USER_REL); - } else if (entity instanceof GroupEntity) { - GroupEntity groupEntity = (GroupEntity) entity; - invalidateRoleGranteeRelations( - groupEntity.namespace().level(0), - groupEntity.roleNames(), - SupportsRelationOperations.Type.ROLE_GROUP_REL); - } - } - - /** - * Invalidates the {@code ROLE_USER_REL} / {@code ROLE_GROUP_REL} cache entries keyed by each role - * the grantee (user/group) is aggregated against. - */ - private void invalidateRoleGranteeRelations( - String metalake, List roleNames, SupportsRelationOperations.Type relType) { - if (roleNames == null || roleNames.isEmpty()) { - return; - } - for (String roleName : roleNames) { - cache.invalidateRelationEntry( - NameIdentifier.of(NamespaceUtil.ofRole(metalake), roleName), - Entity.EntityType.ROLE, - relType); - } - } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/JobMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/JobMetaService.java index 85b33548eca..c325c0115a0 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/JobMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/JobMetaService.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.NameIdentifier; @@ -32,8 +33,10 @@ import org.apache.gravitino.job.JobHandle; import org.apache.gravitino.meta.JobEntity; import org.apache.gravitino.metrics.Monitored; +import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; import org.apache.gravitino.storage.relational.mapper.JobMetaMapper; import org.apache.gravitino.storage.relational.po.JobPO; +import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NamespaceUtil; @@ -111,13 +114,30 @@ public void insertJob(JobEntity jobEntity, boolean overwrite) throws IOException JobPO.JobPOBuilder builder = JobPO.builder().withMetalakeId(metalakeId); JobPO jobPO = JobPO.initializeJobPO(jobEntity, builder); - SessionUtils.doWithCommit( - JobMetaMapper.class, - mapper -> { + SessionUtils.doMultipleWithCommit( + () -> + SessionUtils.doWithoutCommit( + JobMetaMapper.class, + mapper -> { + if (overwrite) { + mapper.insertJobMetaOnDuplicateKeyUpdate(jobPO); + } else { + mapper.insertJobMeta(jobPO); + } + }), + () -> { + // An overwrite is an in-place status update of an existing job, so emit an ALTER row in + // the same transaction to invalidate other nodes' cached copies. A plain insert + // (create) needs no row: list bypasses the cache and there is no negative caching. if (overwrite) { - mapper.insertJobMetaOnDuplicateKeyUpdate(jobPO); - } else { - mapper.insertJobMeta(jobPO); + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.JOB.name(), + jobEntity.nameIdentifier().toString(), + OperateType.ALTER)); } }); } catch (RuntimeException e) { @@ -127,11 +147,31 @@ public void insertJob(JobEntity jobEntity, boolean overwrite) throws IOException @Monitored(metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteJob") public boolean deleteJob(NameIdentifier jobIdent) { + String metalakeName = jobIdent.namespace().level(0); long jobRunIdLong = parseJobRunId(jobIdent.name()); - int result = - SessionUtils.doWithCommitAndFetchResult( - JobMetaMapper.class, mapper -> mapper.softDeleteJobMetaByRunId(jobRunIdLong)); - return result > 0; + AtomicInteger deleteResult = new AtomicInteger(0); + SessionUtils.doMultipleWithCommit( + () -> { + Integer result = + SessionUtils.getWithoutCommit( + JobMetaMapper.class, mapper -> mapper.softDeleteJobMetaByRunId(jobRunIdLong)); + deleteResult.set(result == null ? 0 : result); + }, + () -> { + // Emit a DROP change-log row in the same transaction so other nodes drop their cached + // copy of this job within one poll interval. + if (deleteResult.get() > 0) { + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.JOB.name(), + jobIdent.toString(), + OperateType.DROP)); + } + }); + return deleteResult.get() > 0; } @Monitored( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java index 0c623f96c0a..2c9e8ffe951 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import org.apache.gravitino.Entity; @@ -38,12 +39,14 @@ import org.apache.gravitino.meta.GenericEntity; import org.apache.gravitino.meta.PolicyEntity; import org.apache.gravitino.metrics.Monitored; +import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; import org.apache.gravitino.storage.relational.mapper.PolicyMetaMapper; import org.apache.gravitino.storage.relational.mapper.PolicyMetadataObjectRelMapper; import org.apache.gravitino.storage.relational.mapper.PolicyVersionMapper; import org.apache.gravitino.storage.relational.po.PolicyMaxVersionPO; import org.apache.gravitino.storage.relational.po.PolicyMetadataObjectRelPO; import org.apache.gravitino.storage.relational.po.PolicyPO; +import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; @@ -151,6 +154,8 @@ public PolicyEntity updatePolicy( PolicyPO newPolicyPO = POConverters.updatePolicyPOWithVersion( oldPolicyPO, updatedPolicyEntity, checkNeedUpdateVersion); + // Write the update and its ALTER change-log row (for cross-node cache invalidation) in the + // same transaction, so another node's poller never sees the change without the committed row. if (checkNeedUpdateVersion) { SessionUtils.doMultipleWithCommit( () -> @@ -160,14 +165,41 @@ public PolicyEntity updatePolicy( () -> SessionUtils.doWithoutCommit( PolicyMetaMapper.class, - mapper -> mapper.updatePolicyMeta(newPolicyPO, oldPolicyPO))); + mapper -> mapper.updatePolicyMeta(newPolicyPO, oldPolicyPO)), + () -> + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.POLICY.name(), + ident.toString(), + OperateType.ALTER))); // we set the updateResult to 1 to indicate that the update is successful updateResult = 1; } else { - updateResult = - SessionUtils.doWithCommitAndFetchResult( - PolicyMetaMapper.class, - mapper -> mapper.updatePolicyMeta(newPolicyPO, oldPolicyPO)); + AtomicInteger metaUpdateResult = new AtomicInteger(0); + SessionUtils.doMultipleWithCommit( + () -> { + Integer result = + SessionUtils.getWithoutCommit( + PolicyMetaMapper.class, + mapper -> mapper.updatePolicyMeta(newPolicyPO, oldPolicyPO)); + metaUpdateResult.set(result == null ? 0 : result); + }, + () -> { + if (metaUpdateResult.get() > 0) { + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.POLICY.name(), + ident.toString(), + OperateType.ALTER)); + } + }); + updateResult = metaUpdateResult.get(); } } catch (RuntimeException re) { ExceptionUtils.checkSQLException( @@ -204,7 +236,21 @@ public boolean deletePolicy(NameIdentifier ident) { PolicyVersionMapper.class, mapper -> mapper.softDeletePolicyVersionByMetalakeAndPolicyName( - metalakeName, ident.name()))); + metalakeName, ident.name())), + () -> { + // Emit a DROP change-log row in the same transaction so other nodes drop their cached + // copy of this policy within one poll interval. + if (policyMetaDeletedCount[0] > 0) { + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.POLICY.name(), + ident.toString(), + OperateType.DROP)); + } + }); return policyMetaDeletedCount[0] + policyVersionDeletedCount[0] > 0; } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java index b56f703d2c2..f271b316f0d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import org.apache.gravitino.Entity; @@ -41,10 +42,12 @@ import org.apache.gravitino.meta.GenericEntity; import org.apache.gravitino.meta.TagEntity; import org.apache.gravitino.metrics.Monitored; +import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; import org.apache.gravitino.storage.relational.mapper.TagMetaMapper; import org.apache.gravitino.storage.relational.mapper.TagMetadataObjectRelMapper; import org.apache.gravitino.storage.relational.po.TagMetadataObjectRelPO; import org.apache.gravitino.storage.relational.po.TagPO; +import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; @@ -124,14 +127,33 @@ public TagEntity updateTag( updatedTagEntity.id(), oldTagEntity.id()); - Integer result = - SessionUtils.doWithCommitAndFetchResult( - TagMetaMapper.class, - mapper -> - mapper.updateTagMeta( - POConverters.updateTagPOWithVersion(tagPO, updatedTagEntity), tagPO)); + // Write the update and its change-log row (for cross-node cache invalidation) in the same + // transaction, so another node's poller never sees the ALTER without the committed change. + AtomicInteger updateResult = new AtomicInteger(0); + SessionUtils.doMultipleWithCommit( + () -> { + Integer result = + SessionUtils.getWithoutCommit( + TagMetaMapper.class, + mapper -> + mapper.updateTagMeta( + POConverters.updateTagPOWithVersion(tagPO, updatedTagEntity), tagPO)); + updateResult.set(result == null ? 0 : result); + }, + () -> { + if (updateResult.get() > 0) { + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.TAG.name(), + identifier.toString(), + OperateType.ALTER)); + } + }); - if (result == null || result == 0) { + if (updateResult.get() == 0) { throw new IOException("Failed to update the entity: " + identifier); } @@ -163,7 +185,21 @@ public boolean deleteTag(NameIdentifier identifier) { TagMetadataObjectRelMapper.class, mapper -> mapper.softDeleteTagMetadataObjectRelsByMetalakeAndTagName( - metalakeName, identifier.name()))); + metalakeName, identifier.name())), + () -> { + // Emit a DROP change-log row in the same transaction so other nodes drop their cached + // copy of this tag within one poll interval. + if (tagDeletedCount[0] > 0) { + SessionUtils.doWithoutCommit( + EntityChangeLogMapper.class, + mapper -> + mapper.insertEntityChange( + metalakeName, + Entity.EntityType.TAG.name(), + identifier.toString(), + OperateType.DROP)); + } + }); return tagDeletedCount[0] + tagMetadataObjectRelDeletedCount[0] > 0; } diff --git a/core/src/test/java/org/apache/gravitino/cache/TestCacheConfig.java b/core/src/test/java/org/apache/gravitino/cache/TestCacheConfig.java index f9dbb077be2..1f9e422dfbc 100644 --- a/core/src/test/java/org/apache/gravitino/cache/TestCacheConfig.java +++ b/core/src/test/java/org/apache/gravitino/cache/TestCacheConfig.java @@ -23,7 +23,6 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.collect.ImmutableMap; import java.time.Duration; -import java.util.List; import java.util.stream.IntStream; import org.apache.gravitino.Catalog; import org.apache.gravitino.Config; @@ -62,7 +61,7 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { Caffeine builder = Caffeine.newBuilder(); builder.maximumWeight(5000); builder.weigher(EntityCacheWeigher.getInstance()); - Cache> cache = builder.build(); + Cache cache = builder.build(); BaseMetalake baseMetalake = BaseMetalake.builder() @@ -72,8 +71,8 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withAuditInfo(AuditInfo.EMPTY) .build(); cache.put( - EntityCacheRelationKey.of(NameIdentifier.of("metalake1"), Entity.EntityType.METALAKE), - List.of(baseMetalake)); + EntityCacheKey.of(NameIdentifier.of("metalake1"), Entity.EntityType.METALAKE), + baseMetalake); CatalogEntity catalogEntity = CatalogEntity.builder() .withNamespace(Namespace.of("metalake1")) @@ -84,9 +83,9 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withType(Catalog.Type.RELATIONAL) .build(); cache.put( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of(new String[] {"metalake1", "catalog1"}), Entity.EntityType.CATALOG), - List.of(catalogEntity)); + catalogEntity); SchemaEntity schemaEntity = SchemaEntity.builder() @@ -96,10 +95,10 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withId(1000L) .build(); cache.put( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of(new String[] {"metalake1", "catalog1", "schema1"}), Entity.EntityType.SCHEMA), - List.of(schemaEntity)); + schemaEntity); for (int i = 0; i < 5; i++) { String filesetName = "fileset" + i; @@ -113,10 +112,10 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withFilesetType(Fileset.Type.MANAGED) .build(); cache.put( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of(new String[] {"metalake1", "catalog1", "schema1", filesetName}), Entity.EntityType.FILESET), - List.of(fileset)); + fileset); } for (int i = 0; i < 10; i++) { @@ -129,7 +128,7 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withAuditInfo(AuditInfo.EMPTY) .withId((long) (i + 1) * 100_000) .build(); - cache.put(EntityCacheRelationKey.of(tagNameIdent, Entity.EntityType.TAG), List.of(tagEntity)); + cache.put(EntityCacheKey.of(tagNameIdent, Entity.EntityType.TAG), tagEntity); } // The weight of the cache has exceeded 2000, some entities will be evicted if we continue to @@ -146,10 +145,10 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .withFilesetType(Fileset.Type.MANAGED) .build(); cache.put( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of("metalake1", "catalog1", "schema1", filesetName), Entity.EntityType.FILESET), - List.of(fileset)); + fileset); } // Access filesets 5-14 twice to increase their frequency to 5 (insert + 4 gets) @@ -157,7 +156,7 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { for (int i = 5; i < 15; i++) { String filesetName = "fileset" + i; cache.getIfPresent( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of(new String[] {"metalake1", "catalog1", "schema1", filesetName}), Entity.EntityType.FILESET)); } @@ -175,7 +174,7 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .filter( filesetName -> cache.getIfPresent( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of( new String[] {"metalake1", "catalog1", "schema1", filesetName}), Entity.EntityType.FILESET)) @@ -188,8 +187,7 @@ void testPolicyAndTagCacheWeigher() throws InterruptedException { .mapToObj(i -> NameIdentifierUtil.ofTag("metalake", "tag" + i)) .filter( tagNameIdent -> - cache.getIfPresent( - EntityCacheRelationKey.of(tagNameIdent, Entity.EntityType.TAG)) + cache.getIfPresent(EntityCacheKey.of(tagNameIdent, Entity.EntityType.TAG)) != null) .count(); @@ -217,7 +215,7 @@ void testCaffeineCacheWithWeight() throws Exception { Caffeine builder = Caffeine.newBuilder(); builder.maximumWeight(5000); builder.weigher(EntityCacheWeigher.getInstance()); - Cache> cache = builder.build(); + Cache cache = builder.build(); // Insert 3 metalakes for (int i = 0; i < 3; i++) { @@ -229,8 +227,8 @@ void testCaffeineCacheWithWeight() throws Exception { .withAuditInfo(AuditInfo.EMPTY) .build(); cache.put( - EntityCacheRelationKey.of(NameIdentifier.of("metalake" + i), Entity.EntityType.METALAKE), - List.of(baseMetalake)); + EntityCacheKey.of(NameIdentifier.of("metalake" + i), Entity.EntityType.METALAKE), + baseMetalake); } // Insert 10 catalogs @@ -245,9 +243,8 @@ void testCaffeineCacheWithWeight() throws Exception { .withType(Catalog.Type.RELATIONAL) .build(); cache.put( - EntityCacheRelationKey.of( - NameIdentifier.of("metalake1.catalog" + i), Entity.EntityType.CATALOG), - List.of(catalogEntity)); + EntityCacheKey.of(NameIdentifier.of("metalake1.catalog" + i), Entity.EntityType.CATALOG), + catalogEntity); } // insert 100 schemas @@ -261,24 +258,23 @@ void testCaffeineCacheWithWeight() throws Exception { .build(); cache.put( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of("metalake1.catalog1.schema" + i), Entity.EntityType.SCHEMA), - List.of(schemaEntity)); + schemaEntity); } // Three 3 metalakes still in cache. for (int i = 0; i < 3; i++) { Assertions.assertNotNull( cache.getIfPresent( - EntityCacheRelationKey.of( - NameIdentifier.of("metalake" + 1), Entity.EntityType.METALAKE))); + EntityCacheKey.of(NameIdentifier.of("metalake" + 1), Entity.EntityType.METALAKE))); } // 10 catalogs still in cache. for (int i = 0; i < 10; i++) { Assertions.assertNotNull( cache.getIfPresent( - EntityCacheRelationKey.of( + EntityCacheKey.of( NameIdentifier.of("metalake1.catalog" + i), Entity.EntityType.CATALOG))); } diff --git a/core/src/test/java/org/apache/gravitino/cache/TestCacheIndex.java b/core/src/test/java/org/apache/gravitino/cache/TestCacheIndex.java index cea1f2867a4..91d55dc2508 100644 --- a/core/src/test/java/org/apache/gravitino/cache/TestCacheIndex.java +++ b/core/src/test/java/org/apache/gravitino/cache/TestCacheIndex.java @@ -26,14 +26,13 @@ import java.util.List; import org.apache.gravitino.Entity; import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.utils.NameIdentifierUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class TestCacheIndex { - private RadixTree indexTree; + private RadixTree indexTree; private NameIdentifier ident1; private NameIdentifier ident2; @@ -48,18 +47,18 @@ public class TestCacheIndex { private NameIdentifier ident11; private NameIdentifier ident12; - private EntityCacheRelationKey key1; - private EntityCacheRelationKey key2; - private EntityCacheRelationKey key3; - private EntityCacheRelationKey key4; - private EntityCacheRelationKey key5; - private EntityCacheRelationKey key6; - private EntityCacheRelationKey key7; - private EntityCacheRelationKey key8; - private EntityCacheRelationKey key9; - private EntityCacheRelationKey key10; - private EntityCacheRelationKey key11; - private EntityCacheRelationKey key12; + private EntityCacheKey key1; + private EntityCacheKey key2; + private EntityCacheKey key3; + private EntityCacheKey key4; + private EntityCacheKey key5; + private EntityCacheKey key6; + private EntityCacheKey key7; + private EntityCacheKey key8; + private EntityCacheKey key9; + private EntityCacheKey key10; + private EntityCacheKey key11; + private EntityCacheKey key12; @BeforeEach void setUp() { @@ -80,24 +79,20 @@ void setUp() { ident11 = NameIdentifierUtil.ofUser("metalake2", "user1"); ident12 = NameIdentifierUtil.ofUser("metalake2", "user2"); - key1 = EntityCacheRelationKey.of(ident1, Entity.EntityType.SCHEMA); - key2 = EntityCacheRelationKey.of(ident2, Entity.EntityType.SCHEMA); - key3 = EntityCacheRelationKey.of(ident3, Entity.EntityType.TABLE); - key4 = EntityCacheRelationKey.of(ident4, Entity.EntityType.TOPIC); - key5 = EntityCacheRelationKey.of(ident5, Entity.EntityType.TABLE); - key6 = EntityCacheRelationKey.of(ident6, Entity.EntityType.TABLE); - - key7 = - EntityCacheRelationKey.of( - ident7, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_GROUP_REL); - key8 = - EntityCacheRelationKey.of( - ident8, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_USER_REL); - - key9 = EntityCacheRelationKey.of(ident9, Entity.EntityType.GROUP); - key10 = EntityCacheRelationKey.of(ident10, Entity.EntityType.GROUP); - key11 = EntityCacheRelationKey.of(ident11, Entity.EntityType.USER); - key12 = EntityCacheRelationKey.of(ident12, Entity.EntityType.USER); + key1 = EntityCacheKey.of(ident1, Entity.EntityType.SCHEMA); + key2 = EntityCacheKey.of(ident2, Entity.EntityType.SCHEMA); + key3 = EntityCacheKey.of(ident3, Entity.EntityType.TABLE); + key4 = EntityCacheKey.of(ident4, Entity.EntityType.TOPIC); + key5 = EntityCacheKey.of(ident5, Entity.EntityType.TABLE); + key6 = EntityCacheKey.of(ident6, Entity.EntityType.TABLE); + + key7 = EntityCacheKey.of(ident7, Entity.EntityType.ROLE); + key8 = EntityCacheKey.of(ident8, Entity.EntityType.ROLE); + + key9 = EntityCacheKey.of(ident9, Entity.EntityType.GROUP); + key10 = EntityCacheKey.of(ident10, Entity.EntityType.GROUP); + key11 = EntityCacheKey.of(ident11, Entity.EntityType.USER); + key12 = EntityCacheKey.of(ident12, Entity.EntityType.USER); addIndex(indexTree, key12); addIndex(indexTree, key11); @@ -120,110 +115,106 @@ void testAddIndex() { @Test void testGetFromByMetalakePrefix() { - List storeEntityCacheRelationKeys = + List cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1")); - Assertions.assertEquals(8, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key1)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key3)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key4)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key5)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key6)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key7)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key9)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key10)); - - List storeEntityCacheRelationKeys2 = + Assertions.assertEquals(8, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key1)); + Assertions.assertTrue(cacheKeys.contains(key3)); + Assertions.assertTrue(cacheKeys.contains(key4)); + Assertions.assertTrue(cacheKeys.contains(key5)); + Assertions.assertTrue(cacheKeys.contains(key6)); + Assertions.assertTrue(cacheKeys.contains(key7)); + Assertions.assertTrue(cacheKeys.contains(key9)); + Assertions.assertTrue(cacheKeys.contains(key10)); + + List cacheKeys2 = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake2")); - Assertions.assertEquals(4, storeEntityCacheRelationKeys2.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys2.contains(key2)); - Assertions.assertTrue(storeEntityCacheRelationKeys2.contains(key8)); - Assertions.assertTrue(storeEntityCacheRelationKeys2.contains(key11)); - Assertions.assertTrue(storeEntityCacheRelationKeys2.contains(key12)); + Assertions.assertEquals(4, cacheKeys2.size()); + Assertions.assertTrue(cacheKeys2.contains(key2)); + Assertions.assertTrue(cacheKeys2.contains(key8)); + Assertions.assertTrue(cacheKeys2.contains(key11)); + Assertions.assertTrue(cacheKeys2.contains(key12)); } @Test void testGetByCatalogPrefix() { - List storeEntityCacheRelationKeys = + List cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog1")); - Assertions.assertEquals(4, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key1)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key3)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key4)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key6)); + Assertions.assertEquals(4, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key1)); + Assertions.assertTrue(cacheKeys.contains(key3)); + Assertions.assertTrue(cacheKeys.contains(key4)); + Assertions.assertTrue(cacheKeys.contains(key6)); - storeEntityCacheRelationKeys = - ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog2")); - Assertions.assertEquals(1, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key5)); + cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog2")); + Assertions.assertEquals(1, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key5)); } @Test void testGetBySchemaPrefix() { - List storeEntityCacheRelationKeys = + List cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog1.schema1")); - Assertions.assertEquals(3, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key1)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key3)); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key4)); + Assertions.assertEquals(3, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key1)); + Assertions.assertTrue(cacheKeys.contains(key3)); + Assertions.assertTrue(cacheKeys.contains(key4)); - storeEntityCacheRelationKeys = + cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog1.schema2")); - Assertions.assertEquals(1, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key6)); + Assertions.assertEquals(1, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key6)); - storeEntityCacheRelationKeys = + cacheKeys = ImmutableList.copyOf(indexTree.getValuesForKeysStartingWith("metalake1.catalog2.schema1")); - Assertions.assertEquals(1, storeEntityCacheRelationKeys.size()); - Assertions.assertTrue(storeEntityCacheRelationKeys.contains(key5)); + Assertions.assertEquals(1, cacheKeys.size()); + Assertions.assertTrue(cacheKeys.contains(key5)); } @Test void testGetByExactKey() { - EntityCacheRelationKey storeEntityCacheRelationKey = - indexTree.getValueForExactKey(key1.toString()); - Assertions.assertEquals(key1, storeEntityCacheRelationKey); + EntityCacheKey storeEntityCacheKey = indexTree.getValueForExactKey(key1.toString()); + Assertions.assertEquals(key1, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key2.toString()); - Assertions.assertEquals(key2, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key2.toString()); + Assertions.assertEquals(key2, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key3.toString()); - Assertions.assertEquals(key3, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key3.toString()); + Assertions.assertEquals(key3, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key4.toString()); - Assertions.assertEquals(key4, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key4.toString()); + Assertions.assertEquals(key4, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key5.toString()); - Assertions.assertEquals(key5, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key5.toString()); + Assertions.assertEquals(key5, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key6.toString()); - Assertions.assertEquals(key6, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key6.toString()); + Assertions.assertEquals(key6, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key7.toString()); - Assertions.assertEquals(key7, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key7.toString()); + Assertions.assertEquals(key7, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key8.toString()); - Assertions.assertEquals(key8, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key8.toString()); + Assertions.assertEquals(key8, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key9.toString()); - Assertions.assertEquals(key9, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key9.toString()); + Assertions.assertEquals(key9, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key10.toString()); - Assertions.assertEquals(key10, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key10.toString()); + Assertions.assertEquals(key10, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key11.toString()); - Assertions.assertEquals(key11, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key11.toString()); + Assertions.assertEquals(key11, storeEntityCacheKey); - storeEntityCacheRelationKey = indexTree.getValueForExactKey(key12.toString()); - Assertions.assertEquals(key12, storeEntityCacheRelationKey); + storeEntityCacheKey = indexTree.getValueForExactKey(key12.toString()); + Assertions.assertEquals(key12, storeEntityCacheKey); } - private void addIndex( - RadixTree indexTree, - EntityCacheRelationKey storeEntityCacheRelationKey) { - indexTree.put(storeEntityCacheRelationKey.toString(), storeEntityCacheRelationKey); + private void addIndex(RadixTree indexTree, EntityCacheKey storeEntityCacheKey) { + indexTree.put(storeEntityCacheKey.toString(), storeEntityCacheKey); } } diff --git a/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java b/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java index 58c0450852a..f9a21039fa7 100644 --- a/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java +++ b/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java @@ -16,361 +16,169 @@ * specific language governing permissions and limitations * under the License. */ + package org.apache.gravitino.cache; -import com.google.common.collect.Lists; -import java.time.Instant; -import java.util.List; -import java.util.Optional; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import org.apache.gravitino.Config; import org.apache.gravitino.Entity; -import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; -import org.apache.gravitino.authorization.AuthorizationUtils; -import org.apache.gravitino.authorization.Privileges; -import org.apache.gravitino.authorization.SecurableObject; -import org.apache.gravitino.authorization.SecurableObjects; -import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.ModelVersionEntity; import org.apache.gravitino.meta.RoleEntity; -import org.apache.gravitino.storage.RandomIdGenerator; -import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.meta.SchemaEntity; +import org.apache.gravitino.meta.TableEntity; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.meta.UserEntity; +import org.apache.gravitino.utils.TestUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -/** - * Unit tests for {@link CaffeineEntityCache} invalidation logic, specifically covering the bug - * where invalidating a role entity did not propagate to the METADATA_OBJECT_ROLE_REL relation cache - * entries for the role's securable objects. - * - *

See GitHub issue #11297: {@code listBindingRoleNames} returns stale role after {@code - * revokePrivilegesFromRole} removes the last privilege. - */ +/** Tests hierarchical invalidation and cacheability rules of {@link CaffeineEntityCache}. */ public class TestCaffeineEntityCacheInvalidation { private CaffeineEntityCache cache; - private AuditInfo auditInfo; @BeforeEach void setUp() { - Config config = new Config(false) {}; - cache = new CaffeineEntityCache(config); - auditInfo = AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build(); + cache = new CaffeineEntityCache(new Config() {}); } - /** - * Builds a RoleEntity that has the given schema as its securable object. - * - * @param metalake the metalake name - * @param roleName the role name - * @param catalogName the catalog the schema belongs to - * @param schemaName the schema name used as the role's securable object - * @return the constructed RoleEntity - */ - private RoleEntity buildRoleWithSchemaObject( - String metalake, String roleName, String catalogName, String schemaName) { - SecurableObject catalogObject = SecurableObjects.ofCatalog(catalogName, Lists.newArrayList()); - SecurableObject schemaObject = - SecurableObjects.ofSchema( - catalogObject, schemaName, Lists.newArrayList(Privileges.UseSchema.allow())); - return RoleEntity.builder() - .withId(RandomIdGenerator.INSTANCE.nextId()) - .withName(roleName) - .withNamespace(AuthorizationUtils.ofRoleNamespace(metalake)) - .withProperties(null) - .withAuditInfo(auditInfo) - .withSecurableObjects(Lists.newArrayList(schemaObject)) - .build(); + @Test + void testInvalidateCatalogCascadesToChildren() { + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + SchemaEntity schema = + TestUtil.getTestSchemaEntity(2L, "schema1", Namespace.of("metalake", "catalog1"), "cmt"); + TableEntity table = + TestUtil.getTestTableEntity(3L, "table1", Namespace.of("metalake", "catalog1", "schema1")); + + cache.put(catalog); + cache.put(schema); + cache.put(table); + Assertions.assertEquals(3, cache.size()); + + cache.invalidate(catalog.nameIdentifier(), Entity.EntityType.CATALOG); + + Assertions.assertFalse(cache.contains(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + Assertions.assertFalse(cache.contains(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + Assertions.assertFalse(cache.contains(table.nameIdentifier(), Entity.EntityType.TABLE)); + Assertions.assertEquals(0, cache.size()); } - /** - * Core scenario reproducing GitHub issue #11297: after caching the METADATA_OBJECT_ROLE_REL - * relation for a schema, invalidating the role entity must also remove the stale relation cache. - * - *

Flow: grant → listBindingRoleNames (caches relation) → revoke (invalidates role) → - * listBindingRoleNames must miss cache and re-query. - */ @Test - void testRoleInvalidationPropagatesToMetadataObjectRoleRelCache() { - String metalake = "metalake"; - String catalogName = "catalog"; - String schemaName = "test_schema"; - String roleName = "test_role"; - - RoleEntity role = buildRoleWithSchemaObject(metalake, roleName, catalogName, schemaName); - NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalogName, schemaName); - NameIdentifier roleIdent = NameIdentifierUtil.ofRole(metalake, roleName); + void testInvalidateSchemaKeepsParentCatalog() { + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + SchemaEntity schema = + TestUtil.getTestSchemaEntity(2L, "schema1", Namespace.of("metalake", "catalog1"), "cmt"); + TableEntity table = + TestUtil.getTestTableEntity(3L, "table1", Namespace.of("metalake", "catalog1", "schema1")); + + cache.put(catalog); + cache.put(schema); + cache.put(table); + + cache.invalidate(schema.nameIdentifier(), Entity.EntityType.SCHEMA); + + Assertions.assertTrue(cache.contains(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + Assertions.assertFalse(cache.contains(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + Assertions.assertFalse(cache.contains(table.nameIdentifier(), Entity.EntityType.TABLE)); + } - // Simulate caching the METADATA_OBJECT_ROLE_REL result (e.g., after listBindingRoleNames) - cache.put( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); + @Test + void testInvalidateDoesNotEvictSiblingsWithSharedNamePrefix() { + CatalogEntity catalog1 = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + CatalogEntity catalog10 = + TestUtil.getTestCatalogEntity(2L, "catalog10", Namespace.of("metalake"), "hive", "cmt"); - // Verify the relation cache is populated - Optional> cached = - cache.getIfPresent( - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - schemaIdent, - Entity.EntityType.SCHEMA); - Assertions.assertTrue(cached.isPresent(), "Relation cache should be populated after put"); - Assertions.assertEquals(1, cached.get().size()); - Assertions.assertEquals(roleName, cached.get().get(0).name()); + cache.put(catalog1); + cache.put(catalog10); - // Simulate revokePrivilegesFromRole: invalidate the role entity - cache.invalidate(roleIdent, Entity.EntityType.ROLE); + cache.invalidate(catalog1.nameIdentifier(), Entity.EntityType.CATALOG); - // After invalidation the METADATA_OBJECT_ROLE_REL cache for the schema must be gone - Optional> afterRevoke = - cache.getIfPresent( - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - schemaIdent, - Entity.EntityType.SCHEMA); - Assertions.assertFalse( - afterRevoke.isPresent(), - "METADATA_OBJECT_ROLE_REL cache must be invalidated after revoking the role"); + Assertions.assertFalse(cache.contains(catalog1.nameIdentifier(), Entity.EntityType.CATALOG)); + Assertions.assertTrue(cache.contains(catalog10.nameIdentifier(), Entity.EntityType.CATALOG)); } - /** - * When the role entity was also separately cached (via a previous get), invalidating it must - * still propagate to the METADATA_OBJECT_ROLE_REL relation cache. - */ @Test - void testRoleInvalidationWithSeparatelyCachedRoleEntity() { - String metalake = "metalake"; - String catalogName = "catalog"; - String schemaName = "schema1"; - String roleName = "role1"; + void testRoleUserGroupAreNotCached() { + RoleEntity role = TestUtil.getTestRoleEntity(); + UserEntity user = TestUtil.getTestUserEntity(); + GroupEntity group = TestUtil.getTestGroupEntity(); - RoleEntity role = buildRoleWithSchemaObject(metalake, roleName, catalogName, schemaName); - NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalogName, schemaName); - NameIdentifier roleIdent = NameIdentifierUtil.ofRole(metalake, roleName); - - // Simulate separate entity get (e.g., loadRole) cache.put(role); + cache.put(user); + cache.put(group); - // Simulate relation cache populated after listBindingRoleNames - cache.put( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); - - // Both caches are present - Assertions.assertTrue(cache.contains(roleIdent, Entity.EntityType.ROLE)); + Assertions.assertEquals(0, cache.size()); + Assertions.assertFalse(cache.contains(role.nameIdentifier(), Entity.EntityType.ROLE)); + Assertions.assertFalse(cache.contains(user.nameIdentifier(), Entity.EntityType.USER)); + Assertions.assertFalse(cache.contains(group.nameIdentifier(), Entity.EntityType.GROUP)); Assertions.assertTrue( - cache.contains( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - - // Invalidate role (simulates revokePrivilegesFromRole) - cache.invalidate(roleIdent, Entity.EntityType.ROLE); - - // Both the role entity cache and the relation cache must be gone - Assertions.assertFalse( - cache.contains(roleIdent, Entity.EntityType.ROLE), - "Role entity cache must be gone after invalidation"); - Assertions.assertFalse( - cache.contains( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL), - "METADATA_OBJECT_ROLE_REL cache must be invalidated after revoking the role"); + cache.getIfPresent(role.nameIdentifier(), Entity.EntityType.ROLE).isEmpty()); } - /** - * When multiple roles share the same schema as a securable object, invalidating one role must - * invalidate the METADATA_OBJECT_ROLE_REL cache for that schema (since the full list is stale). - * - *

Note: The existing BFS propagation logic also cascades from the cleared relation cache back - * to other role entities via the schema's reverse index (the same behaviour that clears stale - * role-entity caches on schema rename). Clearing role2's entity cache is a pre-existing - * performance trade-off, not a correctness bug: role2 will be re-fetched fresh from the DB on the - * next access. - */ @Test - void testMultipleRolesInvalidationForSameSchema() { - String metalake = "metalake"; - String catalogName = "catalog"; - String schemaName = "schema"; - - RoleEntity role1 = buildRoleWithSchemaObject(metalake, "role1", catalogName, schemaName); - RoleEntity role2 = buildRoleWithSchemaObject(metalake, "role2", catalogName, schemaName); - - NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalogName, schemaName); - NameIdentifier role1Ident = NameIdentifierUtil.ofRole(metalake, "role1"); - - // Cache both role entities separately and the relation list - cache.put(role1); - cache.put(role2); - cache.put( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role1, role2)); - - Assertions.assertTrue( - cache.contains( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - - // Revoke role1 - cache.invalidate(role1Ident, Entity.EntityType.ROLE); - - // The METADATA_OBJECT_ROLE_REL cache must be gone — this is the core correctness fix - Assertions.assertFalse( - cache.contains( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL), - "Relation cache must be cleared when any of its roles is revoked"); + void testModelAndModelVersionAreNotCached() { + ModelEntity model = TestUtil.getTestModelEntity(1L, "model1", Namespace.of("m1", "c1", "s1")); + ModelVersionEntity version = + TestUtil.getTestModelVersionEntity( + model.nameIdentifier(), + 1, + ImmutableMap.of("unknown", "uri"), + ImmutableMap.of(), + "cmt", + ImmutableList.of()); + + cache.put(model); + cache.put(version); + + // Model and model version carry a load-bearing pointer (a version URI, the latest version), so + // they are not cached; reads go straight to the store. + Assertions.assertEquals(0, cache.size()); + Assertions.assertFalse(cache.contains(model.nameIdentifier(), Entity.EntityType.MODEL)); } - /** - * When a role has securable objects on multiple schemas, invalidating the role must invalidate - * the METADATA_OBJECT_ROLE_REL caches for ALL schemas. - */ @Test - void testRoleWithMultipleSchemasInvalidatesAllRelationCaches() { - String metalake = "metalake"; - String catalogName = "catalog"; - String roleName = "multi_schema_role"; + void testTagIsCached() { + TagEntity tag = TestUtil.getTestTagEntity(); - SecurableObject cat = SecurableObjects.ofCatalog(catalogName, Lists.newArrayList()); - SecurableObject schema1Obj = - SecurableObjects.ofSchema(cat, "schema1", Lists.newArrayList(Privileges.UseSchema.allow())); - SecurableObject schema2Obj = - SecurableObjects.ofSchema(cat, "schema2", Lists.newArrayList(Privileges.UseSchema.allow())); + cache.put(tag); - RoleEntity role = - RoleEntity.builder() - .withId(RandomIdGenerator.INSTANCE.nextId()) - .withName(roleName) - .withNamespace(AuthorizationUtils.ofRoleNamespace(metalake)) - .withProperties(null) - .withAuditInfo(auditInfo) - .withSecurableObjects(Lists.newArrayList(schema1Obj, schema2Obj)) - .build(); - - NameIdentifier schema1Ident = NameIdentifier.of(metalake, catalogName, "schema1"); - NameIdentifier schema2Ident = NameIdentifier.of(metalake, catalogName, "schema2"); - NameIdentifier roleIdent = NameIdentifierUtil.ofRole(metalake, roleName); - - // Cache METADATA_OBJECT_ROLE_REL for both schemas - cache.put( - schema1Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); - cache.put( - schema2Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); - - Assertions.assertTrue( - cache.contains( - schema1Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - Assertions.assertTrue( - cache.contains( - schema2Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - - // Revoke role (removes its last privilege on all schemas) - cache.invalidate(roleIdent, Entity.EntityType.ROLE); - - Assertions.assertFalse( - cache.contains( - schema1Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL), - "schema1 METADATA_OBJECT_ROLE_REL cache must be cleared"); - Assertions.assertFalse( - cache.contains( - schema2Ident, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL), - "schema2 METADATA_OBJECT_ROLE_REL cache must be cleared"); + Assertions.assertTrue(cache.contains(tag.nameIdentifier(), Entity.EntityType.TAG)); + Assertions.assertEquals( + tag, cache.getIfPresent(tag.nameIdentifier(), Entity.EntityType.TAG).orElse(null)); } - /** - * When the METADATA_OBJECT_ROLE_REL cache already contains the role (e.g., role was granted - * earlier and the relation was cached), granting an additional privilege (invalidates the role) - * must also clear the stale relation cache. This is symmetric with the revoke path. - */ @Test - void testGrantPathInvalidatesRelationCacheWhenRoleWasPreviouslyCached() { - String metalake = "metalake"; - String catalogName = "catalog"; - String schemaName = "schema_grant"; - String roleName = "grant_role"; - - RoleEntity role = buildRoleWithSchemaObject(metalake, roleName, catalogName, schemaName); - NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalogName, schemaName); - NameIdentifier roleIdent = NameIdentifierUtil.ofRole(metalake, roleName); - - // Simulate a prior listBindingRoleNames that already included the role - cache.put( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); - - // Simulate grantPrivilegesToRole adding another privilege → role entity is updated and - // invalidated - cache.invalidate(roleIdent, Entity.EntityType.ROLE); - - // Relation cache should be gone, forcing a fresh DB query on next listBindingRoleNames - Optional> afterGrant = - cache.getIfPresent( - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - schemaIdent, - Entity.EntityType.SCHEMA); - Assertions.assertFalse( - afterGrant.isPresent(), - "METADATA_OBJECT_ROLE_REL cache must be cleared after grant so next read returns fresh data"); + void testGetIfPresentReturnsCachedEntity() { + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + cache.put(catalog); + + Assertions.assertEquals( + catalog, + cache.getIfPresent(catalog.nameIdentifier(), Entity.EntityType.CATALOG).orElse(null)); + Assertions.assertTrue( + cache.getIfPresent(catalog.nameIdentifier(), Entity.EntityType.SCHEMA).isEmpty()); } - /** - * Verify the reverse index is cleaned up correctly after invalidation, preventing memory leaks. - */ @Test - void testReverseIndexCleanupAfterRoleInvalidation() { - String metalake = "metalake"; - String catalogName = "catalog"; - String schemaName = "schema_cleanup"; - String roleName = "cleanup_role"; - - RoleEntity role = buildRoleWithSchemaObject(metalake, roleName, catalogName, schemaName); - NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalogName, schemaName); - NameIdentifier roleIdent = NameIdentifierUtil.ofRole(metalake, roleName); + void testClearResetsSizeAndIndex() { + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + cache.put(catalog); + Assertions.assertEquals(1, cache.size()); - cache.put(role); - cache.put( - schemaIdent, - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL, - Lists.newArrayList(role)); - - long sizeBeforeInvalidation = cache.size(); - cache.invalidate(roleIdent, Entity.EntityType.ROLE); - long sizeAfterInvalidation = cache.size(); - - // Both the role entity entry and the relation entry should be removed - Assertions.assertTrue( - sizeAfterInvalidation < sizeBeforeInvalidation, - "Cache size must decrease after invalidating role and its related relation entries"); + cache.clear(); - // Reverse index for role should be empty - ReverseIndexCache reverseIndex = cache.getReverseIndex(); - List roleReverseKeys = reverseIndex.get(roleIdent, Entity.EntityType.ROLE); - Assertions.assertNull( - roleReverseKeys, "Reverse index for role should be empty after invalidation"); + Assertions.assertEquals(0, cache.size()); + Assertions.assertFalse(cache.contains(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); } } diff --git a/core/src/test/java/org/apache/gravitino/cache/TestEntityCacheKey.java b/core/src/test/java/org/apache/gravitino/cache/TestEntityCacheKey.java index 6fbfd08f246..903dcb58191 100644 --- a/core/src/test/java/org/apache/gravitino/cache/TestEntityCacheKey.java +++ b/core/src/test/java/org/apache/gravitino/cache/TestEntityCacheKey.java @@ -21,7 +21,6 @@ import org.apache.gravitino.Entity; import org.apache.gravitino.NameIdentifier; -import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.utils.NameIdentifierUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -29,44 +28,27 @@ public class TestEntityCacheKey { @Test - void testCreateRelationEntityCacheRelationKeyUsingStaticMethod() { + void testCreateEntityCacheKeyUsingStaticMethod() { NameIdentifier ident = NameIdentifierUtil.ofRole("metalake", "role1"); - // test Relation Entity - EntityCacheRelationKey key = - EntityCacheRelationKey.of( - ident, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_GROUP_REL); - Assertions.assertEquals("metalake.system.role.role1:ROLE:ROLE_GROUP_REL", key.toString()); + EntityCacheKey key = EntityCacheKey.of(ident, Entity.EntityType.ROLE); + Assertions.assertEquals("metalake.system.role.role1:ROLE", key.toString()); Assertions.assertEquals( NameIdentifier.of("metalake", "system", "role", "role1"), key.identifier()); Assertions.assertEquals(Entity.EntityType.ROLE, key.entityType()); - Assertions.assertEquals(SupportsRelationOperations.Type.ROLE_GROUP_REL, key.relationType()); - - // test Store Entity - EntityCacheRelationKey key2 = EntityCacheRelationKey.of(ident, Entity.EntityType.ROLE, null); - Assertions.assertEquals("metalake.system.role.role1:ROLE", key2.toString()); - Assertions.assertEquals( - NameIdentifier.of("metalake", "system", "role", "role1"), key2.identifier()); - Assertions.assertEquals(Entity.EntityType.ROLE, key2.entityType()); - Assertions.assertNull(key2.relationType()); } @Test - void testCreateRelationEntityCacheRelationKeyWithNullArguments() { + void testCreateEntityCacheKeyWithNullArguments() { NameIdentifier ident = NameIdentifierUtil.ofRole("metalake", "role1"); Assertions.assertThrows( IllegalArgumentException.class, () -> { - EntityCacheRelationKey.of( - null, Entity.EntityType.ROLE, SupportsRelationOperations.Type.ROLE_GROUP_REL); + EntityCacheKey.of(null, Entity.EntityType.ROLE); }); Assertions.assertThrows( IllegalArgumentException.class, () -> { - EntityCacheRelationKey.of(ident, null, SupportsRelationOperations.Type.ROLE_GROUP_REL); - }); - Assertions.assertDoesNotThrow( - () -> { - EntityCacheRelationKey.of(ident, Entity.EntityType.ROLE, null); + EntityCacheKey.of(ident, null); }); } @@ -75,37 +57,20 @@ public void testEqualsAndHashCodecEquality() { NameIdentifier ident1 = NameIdentifier.of("ns", "db", "tbl"); Entity.EntityType type = Entity.EntityType.TABLE; - EntityCacheRelationKey key1 = EntityCacheRelationKey.of(ident1, type); - EntityCacheRelationKey key2 = - EntityCacheRelationKey.of(NameIdentifier.of("ns", "db", "tbl"), type); + EntityCacheKey key1 = EntityCacheKey.of(ident1, type); + EntityCacheKey key2 = EntityCacheKey.of(NameIdentifier.of("ns", "db", "tbl"), type); Assertions.assertEquals(key1, key2, "Keys with same ident and type should be equal"); Assertions.assertEquals( key1.hashCode(), key2.hashCode(), "Hash codes must match for equal objects"); } - @Test - public void testEqualsAndHashCodeWithRelationType() { - NameIdentifier ident = NameIdentifier.of("ns", "db", "tbl"); - Entity.EntityType type = Entity.EntityType.TABLE; - SupportsRelationOperations.Type relType = SupportsRelationOperations.Type.OWNER_REL; - - EntityCacheRelationKey key1 = EntityCacheRelationKey.of(ident, type, relType); - EntityCacheRelationKey key2 = - EntityCacheRelationKey.of(NameIdentifier.of("ns", "db", "tbl"), type, relType); - - Assertions.assertEquals( - key1, key2, "Keys with same ident, type, and relationType should be equal"); - Assertions.assertEquals( - key1.hashCode(), key2.hashCode(), "Hash codes must match for equal objects"); - } - @Test public void testInequalityWithDifferentIdentifier() { - EntityCacheRelationKey key1 = - EntityCacheRelationKey.of(NameIdentifier.of("ns", "db", "tbl1"), Entity.EntityType.TABLE); - EntityCacheRelationKey key2 = - EntityCacheRelationKey.of(NameIdentifier.of("ns", "db", "tbl2"), Entity.EntityType.TABLE); + EntityCacheKey key1 = + EntityCacheKey.of(NameIdentifier.of("ns", "db", "tbl1"), Entity.EntityType.TABLE); + EntityCacheKey key2 = + EntityCacheKey.of(NameIdentifier.of("ns", "db", "tbl2"), Entity.EntityType.TABLE); Assertions.assertNotEquals(key1, key2, "Keys with different identifiers should not be equal"); } @@ -113,44 +78,19 @@ public void testInequalityWithDifferentIdentifier() { @Test public void testInequalityWithDifferentEntityType() { NameIdentifier ident = NameIdentifier.of("ns", "db", "obj"); - EntityCacheRelationKey key1 = EntityCacheRelationKey.of(ident, Entity.EntityType.TABLE); - EntityCacheRelationKey key2 = EntityCacheRelationKey.of(ident, Entity.EntityType.FILESET); + EntityCacheKey key1 = EntityCacheKey.of(ident, Entity.EntityType.TABLE); + EntityCacheKey key2 = EntityCacheKey.of(ident, Entity.EntityType.FILESET); Assertions.assertNotEquals(key1, key2, "Keys with different entity types should not be equal"); } @Test - public void testInequalityWithDifferentRelationType() { - NameIdentifier ident = NameIdentifier.of("ns", "db", "obj"); - Entity.EntityType type = Entity.EntityType.TABLE; - - EntityCacheRelationKey key1 = - EntityCacheRelationKey.of(ident, type, SupportsRelationOperations.Type.OWNER_REL); - EntityCacheRelationKey key2 = - EntityCacheRelationKey.of(ident, type, SupportsRelationOperations.Type.ROLE_USER_REL); - - Assertions.assertNotEquals( - key1, key2, "Keys with different relation types should not be equal"); - } - - @Test - public void testToStringWithoutRelationType() { + public void testToString() { NameIdentifier ident = NameIdentifierUtil.ofUser("metalake", "user1"); Entity.EntityType type = Entity.EntityType.USER; - EntityCacheRelationKey key = EntityCacheRelationKey.of(ident, type); + EntityCacheKey key = EntityCacheKey.of(ident, type); Assertions.assertEquals("metalake.system.user.user1:USER", key.toString()); } - - @Test - public void testToStringWithRelationType() { - NameIdentifier ident = NameIdentifierUtil.ofUser("metalake", "user1"); - Entity.EntityType type = Entity.EntityType.USER; - SupportsRelationOperations.Type relationType = SupportsRelationOperations.Type.ROLE_USER_REL; - - EntityCacheRelationKey key = EntityCacheRelationKey.of(ident, type, relationType); - - Assertions.assertEquals("metalake.system.user.user1:USER:ROLE_USER_REL", key.toString()); - } } diff --git a/core/src/test/java/org/apache/gravitino/storage/TestEntityStorageRelationCache.java b/core/src/test/java/org/apache/gravitino/storage/TestEntityStorageRelationCache.java index 5bbec6c5e37..396e24b861c 100644 --- a/core/src/test/java/org/apache/gravitino/storage/TestEntityStorageRelationCache.java +++ b/core/src/test/java/org/apache/gravitino/storage/TestEntityStorageRelationCache.java @@ -19,7 +19,6 @@ package org.apache.gravitino.storage; -import com.github.benmanes.caffeine.cache.Cache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import java.time.Instant; @@ -41,10 +40,6 @@ import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.SecurableObjects; -import org.apache.gravitino.cache.CaffeineEntityCache; -import org.apache.gravitino.cache.EntityCacheKey; -import org.apache.gravitino.cache.EntityCacheRelationKey; -import org.apache.gravitino.cache.ReverseIndexCache; import org.apache.gravitino.function.FunctionDefinition; import org.apache.gravitino.function.FunctionDefinitions; import org.apache.gravitino.function.FunctionImpl; @@ -71,7 +66,6 @@ import org.apache.gravitino.rel.Representation; import org.apache.gravitino.rel.SQLRepresentation; import org.apache.gravitino.rel.types.Types; -import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; import org.junit.jupiter.api.Assertions; @@ -211,36 +205,16 @@ void testInvalidRelationCache(String type, boolean enableCache) throws Exception store.delete(readRole.nameIdentifier(), Entity.EntityType.ROLE); - ReverseIndexCache reverseIndexCache = - ((CaffeineEntityCache) ((RelationalEntityStore) store).getCache()).getReverseIndex(); - List reverseIndexValue = - reverseIndexCache.get( - NameIdentifier.of("metalake", "newCatalogName", "schema", "fileset"), - Entity.EntityType.FILESET); - Assertions.assertEquals(1, reverseIndexValue.size()); - Assertions.assertEquals(writeRole.nameIdentifier(), reverseIndexValue.get(0).identifier()); + Role reloadedWriteRole = + store.get(writeRole.nameIdentifier(), Entity.EntityType.ROLE, RoleEntity.class); + Assertions.assertEquals(1, reloadedWriteRole.securableObjects().size()); store.put(readRole, true); store.get(readRole.nameIdentifier(), Entity.EntityType.ROLE, RoleEntity.class); - reverseIndexValue = - reverseIndexCache.get( - NameIdentifier.of("metalake", "newCatalogName", "schema", "fileset"), - Entity.EntityType.FILESET); - Assertions.assertEquals(2, reverseIndexValue.size()); - List ids = - reverseIndexValue.stream().map(EntityCacheKey::identifier).collect(Collectors.toList()); - Assertions.assertTrue(ids.contains(readRole.nameIdentifier())); - Assertions.assertTrue(ids.contains(writeRole.nameIdentifier())); store.delete(readRole.nameIdentifier(), Entity.EntityType.ROLE); store.delete(writeRole.nameIdentifier(), Entity.EntityType.ROLE); - reverseIndexValue = - reverseIndexCache.get( - NameIdentifier.of("metalake", "newCatalogName", "schema", "fileset"), - Entity.EntityType.FILESET); - Assertions.assertNull(reverseIndexValue); - store.put(readRole, true); store.put(writeRole, true); store.get(readRole.nameIdentifier(), Entity.EntityType.ROLE, RoleEntity.class); @@ -374,23 +348,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { Assertions.assertEquals(1, tags.size()); Assertions.assertEquals(tag1, tags.get(0)); - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - CaffeineEntityCache caffeineEntityCache = - (CaffeineEntityCache) relationalEntityStore.getCache(); - Cache> cache = caffeineEntityCache.getCacheData(); - - List cachedTags = - cache.get( - EntityCacheRelationKey.of( - catalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - - Assertions.assertNotNull(cachedTags); - Assertions.assertEquals(1, cachedTags.size()); - Assertions.assertEquals(tag1, cachedTags.get(0)); - List genericEntities = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -417,23 +374,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { CatalogEntity.class, Entity.EntityType.CATALOG, e -> updatedCatalog); - List cachedTagsAfterCatalogUpdate = - cache.get( - EntityCacheRelationKey.of( - catalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNull(cachedTagsAfterCatalogUpdate); - - List cachedTagsByTagAfterCatalogUpdate = - cache.get( - EntityCacheRelationKey.of( - tag1.nameIdentifier(), - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNull(cachedTagsByTagAfterCatalogUpdate); List tagsAfterCatalogUpdate = relationOperations.listEntitiesByRelation( @@ -444,17 +384,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { Assertions.assertEquals(1, tagsAfterCatalogUpdate.size()); Assertions.assertEquals(tag1, tagsAfterCatalogUpdate.get(0)); - List cachedTagsAfterReload = - cache.get( - EntityCacheRelationKey.of( - updatedCatalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNotNull(cachedTagsAfterReload); - Assertions.assertEquals(1, cachedTagsAfterReload.size()); - Assertions.assertEquals(tag1, cachedTagsAfterReload.get(0)); - List genericEntitiesAfterCatalogUpdate = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -466,20 +395,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { Assertions.assertEquals( updatedCatalog.name(), genericEntitiesAfterCatalogUpdate.get(0).name()); - List cachedTagsByTagAfterReload = - cache.get( - EntityCacheRelationKey.of( - tag1.nameIdentifier(), - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNotNull(cachedTagsByTagAfterReload); - Assertions.assertEquals(1, cachedTagsByTagAfterReload.size()); - Assertions.assertEquals( - updatedCatalog.id(), ((GenericEntity) cachedTagsByTagAfterReload.get(0)).id()); - Assertions.assertEquals( - updatedCatalog.name(), ((GenericEntity) cachedTagsByTagAfterReload.get(0)).name()); - TagEntity updatedTag1 = TagEntity.builder() .withId(tag1.id()) @@ -490,24 +405,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { .build(); store.update(tag1.nameIdentifier(), TagEntity.class, Entity.EntityType.TAG, e -> updatedTag1); - List cachedTagsAfterTagUpdate = - cache.get( - EntityCacheRelationKey.of( - updatedCatalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNull(cachedTagsAfterTagUpdate); - - List cachedEntitiesAfterTagUpdate = - cache.get( - EntityCacheRelationKey.of( - tag1.nameIdentifier(), - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNull(cachedEntitiesAfterTagUpdate); - List tagsAfterTagUpdate = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -517,17 +414,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { Assertions.assertEquals(1, tagsAfterTagUpdate.size()); Assertions.assertEquals(updatedTag1, tagsAfterTagUpdate.get(0)); - List cachedTagsAfterTagReload = - cache.get( - EntityCacheRelationKey.of( - updatedCatalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNotNull(cachedTagsAfterTagReload); - Assertions.assertEquals(1, cachedTagsAfterTagReload.size()); - Assertions.assertEquals(updatedTag1, cachedTagsAfterTagReload.get(0)); - List genericEntitiesAfterTagUpdate = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -537,19 +423,6 @@ void testTagRelationCache(String type, boolean enableCache) throws Exception { Assertions.assertEquals(1, genericEntitiesAfterTagUpdate.size()); Assertions.assertEquals(catalog.id(), genericEntitiesAfterTagUpdate.get(0).id()); Assertions.assertEquals(updatedCatalog.name(), genericEntitiesAfterTagUpdate.get(0).name()); - List cachedTagsByTagAfterTagReload = - cache.get( - EntityCacheRelationKey.of( - updatedTag1.nameIdentifier(), - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL), - k -> null); - Assertions.assertNotNull(cachedTagsByTagAfterTagReload); - Assertions.assertEquals(1, cachedTagsByTagAfterTagReload.size()); - Assertions.assertEquals( - updatedCatalog.id(), ((GenericEntity) cachedTagsByTagAfterTagReload.get(0)).id()); - Assertions.assertEquals( - updatedCatalog.name(), ((GenericEntity) cachedTagsByTagAfterTagReload.get(0)).name()); destroy(type); } } @@ -878,16 +751,6 @@ void testFunctionTagRelationCacheInvalidation(String type, boolean enableCache) Entity.EntityType.FUNCTION, e -> renamedFunction); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - ReverseIndexCache reverseIndexCache = cache.getReverseIndex(); - Assertions.assertNull( - reverseIndexCache.get(function.nameIdentifier(), Entity.EntityType.FUNCTION)); - } - } - List tagsAfterRename = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -914,7 +777,7 @@ void testFunctionTagRelationCacheInvalidation(String type, boolean enableCache) @ParameterizedTest @MethodSource("storageProvider") - void testViewNotIndexedInReverseCache(String type, boolean enableCache) throws Exception { + void testViewCachedForSingleEntityLookup(String type, boolean enableCache) throws Exception { Config config = Mockito.mock(Config.class); Mockito.when(config.get(Configs.CACHE_ENABLED)).thenReturn(enableCache); init(type, config); @@ -960,28 +823,16 @@ void testViewNotIndexedInReverseCache(String type, boolean enableCache) throws E .withAuditInfo(auditInfo) .build(); - // Use store.put() to trigger the actual cache + reverse index flow + // Use store.put() to trigger the single-entity cache flow store.put(view, false); - // Verify view IS in forward cache (performance cache for get operations) + // Verify the view can be read back through the store (served from cache when enabled) ViewEntity retrievedView = store.get(view.nameIdentifier(), Entity.EntityType.VIEW, ViewEntity.class); Assertions.assertNotNull(retrievedView); Assertions.assertEquals(view.id(), retrievedView.id()); Assertions.assertEquals(view.name(), retrievedView.name()); - // Get reverse index cache to verify view is NOT indexed - ReverseIndexCache reverseIndexCache = - ((CaffeineEntityCache) ((RelationalEntityStore) store).getCache()).getReverseIndex(); - - // Verify view is NOT in reverse index cache - // Views have namespace and should be skipped by GENERIC_METADATA_OBJECT_REVERSE_RULE - List reverseIndexValue = - reverseIndexCache.get(view.nameIdentifier(), Entity.EntityType.VIEW); - Assertions.assertNull( - reverseIndexValue, - "Views should NOT be indexed in reverse cache - they have namespace and don't support tags/policies"); - destroy(type); } } @@ -1022,25 +873,7 @@ void testCacheInvalidationOnNewRelation(String type, boolean enableCache) throws true); Assertions.assertTrue(tags.isEmpty()); - // 2. Verify cache has empty list if cache is enabled - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - List cachedEntities = - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - catalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL)); - Assertions.assertNotNull(cachedEntities); - Assertions.assertTrue(cachedEntities.isEmpty()); - } - } - - // 3. Create a tag and add relation + // 2. Create a tag and add relation Namespace tagNamespace = NameIdentifierUtil.ofTag("metalake", "tag1").namespace(); TagEntity tag1 = TagEntity.builder() @@ -1077,23 +910,6 @@ void testCacheInvalidationOnNewRelation(String type, boolean enableCache) throws true); Assertions.assertTrue(owners.isEmpty()); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - List cachedOwners = - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - catalog.nameIdentifier(), - Entity.EntityType.CATALOG, - SupportsRelationOperations.Type.OWNER_REL)); - Assertions.assertNotNull(cachedOwners); - Assertions.assertTrue(cachedOwners.isEmpty()); - } - } - UserEntity ownerUser = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), @@ -1172,25 +988,7 @@ void testCacheInvalidationOnNewRelationReverse(String type, boolean enableCache) true); Assertions.assertTrue(entities.isEmpty()); - // 2. Verify cache has empty list if cache is enabled - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - List cachedEntities = - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - tag1.nameIdentifier(), - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL)); - Assertions.assertNotNull(cachedEntities); - Assertions.assertTrue(cachedEntities.isEmpty()); - } - } - - // 3. Add relation from Catalog (Source side) + // 2. Add relation from Catalog (Source side) relationOperations.updateEntityRelations( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, catalog.nameIdentifier(), @@ -1198,7 +996,7 @@ void testCacheInvalidationOnNewRelationReverse(String type, boolean enableCache) new NameIdentifier[] {tag1.nameIdentifier()}, new NameIdentifier[] {}); - // 4. Fetch relation for Tag again, it should NOT be empty + // 3. Fetch relation for Tag again, it should NOT be empty entities = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, @@ -1276,40 +1074,14 @@ void testRoleCacheEvictedOnFunctionDeletion(String type, boolean enableCache) th .build(); store.put(role, false); - // Read role to populate cache; ROLE_SECURABLE_OBJECT_REVERSE_RULE writes: - // funcIdent:FUNCTION -> [roleIdent:ROLE] into the reverse index. + // Read the role once so a stale copy would be observable if it were cached. RoleEntity cachedRole = store.get(role.nameIdentifier(), Entity.EntityType.ROLE, RoleEntity.class); Assertions.assertEquals(3, cachedRole.securableObjects().size()); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - ReverseIndexCache reverseIndex = - ((CaffeineEntityCache) relationalEntityStore.getCache()).getReverseIndex(); - List reverseKeys = - reverseIndex.get(function.nameIdentifier(), Entity.EntityType.FUNCTION); - Assertions.assertNotNull(reverseKeys); - Assertions.assertTrue( - reverseKeys.stream().anyMatch(k -> k.identifier().equals(role.nameIdentifier()))); - } - } - - // Delete the function; BFS invalidation should evict the role from cache via reverse index. + // Delete the function; the next role read must not serve a stale securable object list. store.delete(function.nameIdentifier(), Entity.EntityType.FUNCTION, false); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - Assertions.assertNull( - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of(role.nameIdentifier(), Entity.EntityType.ROLE))); - } - } - // Re-read role; the function securable object should have been removed from DB via cascade. RoleEntity reloadedRole = store.get(role.nameIdentifier(), Entity.EntityType.ROLE, RoleEntity.class); @@ -1379,24 +1151,7 @@ void testFunctionOwnerRelationCacheInvalidation(String type, boolean enableCache true); Assertions.assertTrue(owners.isEmpty()); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - List cachedOwners = - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - function.nameIdentifier(), - Entity.EntityType.FUNCTION, - SupportsRelationOperations.Type.OWNER_REL)); - Assertions.assertNotNull(cachedOwners); - Assertions.assertTrue(cachedOwners.isEmpty()); - } - } - - // 2. Set an owner; cache for OWNER_REL should be invalidated + // 2. Set an owner; the next read must reflect it UserEntity ownerUser = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), @@ -1413,7 +1168,7 @@ void testFunctionOwnerRelationCacheInvalidation(String type, boolean enableCache Entity.EntityType.USER, true); - // 3. Query owner again - should return the owner and be re-cached + // 3. Query owner again - should return the owner owners = relationOperations.listEntitiesByRelation( SupportsRelationOperations.Type.OWNER_REL, @@ -1423,7 +1178,7 @@ void testFunctionOwnerRelationCacheInvalidation(String type, boolean enableCache Assertions.assertEquals(1, owners.size()); Assertions.assertEquals(ownerUser.name(), owners.get(0).name()); - // 4. Rename the function; old OWNER_REL cache entry should be evicted via BFS invalidation + // 4. Rename the function; owner lookups via the new name must keep working FunctionEntity renamedFunction = createFunctionEntity( function.id(), @@ -1436,23 +1191,6 @@ void testFunctionOwnerRelationCacheInvalidation(String type, boolean enableCache Entity.EntityType.FUNCTION, e -> renamedFunction); - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relationalEntityStore = (RelationalEntityStore) store; - if (relationalEntityStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache cache = (CaffeineEntityCache) relationalEntityStore.getCache(); - // The old function's OWNER_REL cache should have been evicted by BFS invalidation - List cachedOwners = - cache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - function.nameIdentifier(), - Entity.EntityType.FUNCTION, - SupportsRelationOperations.Type.OWNER_REL)); - Assertions.assertNull(cachedOwners); - } - } - // 5. Query owner via new name - should still return the correct owner owners = relationOperations.listEntitiesByRelation( @@ -1540,27 +1278,8 @@ void testRevokePrivilegeInvalidatesMetadataObjectRoleRelCache(String type, boole Assertions.assertEquals(1, rolesBeforeRevoke.size()); Assertions.assertEquals("test_role", rolesBeforeRevoke.get(0).name()); - // Verify the relation is in cache when cache is enabled - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relStore = (RelationalEntityStore) store; - if (relStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache caffeineCache = (CaffeineEntityCache) relStore.getCache(); - List cachedRoles = - caffeineCache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - schema.nameIdentifier(), - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - Assertions.assertNotNull(cachedRoles, "Cache should be populated after first fetch"); - Assertions.assertEquals(1, cachedRoles.size()); - } - } - // Simulate revokePrivilegesFromRole: update the role to remove the schema securable object. - // RelationalEntityStore.update() calls cache.invalidate(roleIdent, ROLE) afterwards, - // which must also invalidate the schema's METADATA_OBJECT_ROLE_REL cache entry. + // The next listEntitiesByRelation must reflect the change immediately. store.update( role.nameIdentifier(), RoleEntity.class, @@ -1575,25 +1294,6 @@ void testRevokePrivilegeInvalidatesMetadataObjectRoleRelCache(String type, boole .withSecurableObjects(Lists.newArrayList()) // all privileges revoked .build()); - // Verify METADATA_OBJECT_ROLE_REL cache is gone when cache is enabled - if (enableCache && store instanceof RelationalEntityStore) { - RelationalEntityStore relStore = (RelationalEntityStore) store; - if (relStore.getCache() instanceof CaffeineEntityCache) { - CaffeineEntityCache caffeineCache = (CaffeineEntityCache) relStore.getCache(); - List cachedAfterRevoke = - caffeineCache - .getCacheData() - .getIfPresent( - EntityCacheRelationKey.of( - schema.nameIdentifier(), - Entity.EntityType.SCHEMA, - SupportsRelationOperations.Type.METADATA_OBJECT_ROLE_REL)); - Assertions.assertNull( - cachedAfterRevoke, - "METADATA_OBJECT_ROLE_REL cache must be invalidated after role update"); - } - } - // The key correctness check: listEntitiesByRelation must not return the revoked role List rolesAfterRevoke = relationOperations.listEntitiesByRelation( diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheChangeLogListener.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheChangeLogListener.java new file mode 100644 index 00000000000..6fb5f5f87dc --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheChangeLogListener.java @@ -0,0 +1,175 @@ +/* + * 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.storage.relational; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.List; +import org.apache.gravitino.Config; +import org.apache.gravitino.Entity.EntityType; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.cache.CaffeineEntityCache; +import org.apache.gravitino.cache.Coherence; +import org.apache.gravitino.cache.EntityCache; +import org.apache.gravitino.cache.NoOpsCache; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaEntity; +import org.apache.gravitino.meta.TableEntity; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord; +import org.apache.gravitino.storage.relational.po.cache.OperateType; +import org.apache.gravitino.utils.TestUtil; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** Tests {@link EntityCacheChangeLogListener}: parsing, dispatch, hierarchical cascade. */ +public class TestEntityCacheChangeLogListener { + + private static EntityChangeRecord record( + EntityType type, String fullName, OperateType operateType) { + return new EntityChangeRecord( + 1L, "metalake", type == null ? null : type.name(), fullName, operateType, 0L); + } + + @Test + void testDropInvalidatesExactlyTheChangedKey() { + CaffeineEntityCache cache = new CaffeineEntityCache(new Config() {}); + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + SchemaEntity schema = + TestUtil.getTestSchemaEntity(2L, "schema1", Namespace.of("metalake", "catalog1"), "cmt"); + cache.put(catalog); + cache.put(schema); + + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + listener.onEntityChange( + List.of(record(EntityType.SCHEMA, schema.nameIdentifier().toString(), OperateType.DROP))); + + Assertions.assertFalse(cache.contains(schema.nameIdentifier(), EntityType.SCHEMA)); + Assertions.assertTrue(cache.contains(catalog.nameIdentifier(), EntityType.CATALOG)); + } + + @Test + void testContainerDropCascadesToChildrenButKeepsSiblings() { + CaffeineEntityCache cache = new CaffeineEntityCache(new Config() {}); + CatalogEntity catalog = + TestUtil.getTestCatalogEntity(1L, "catalog1", Namespace.of("metalake"), "hive", "cmt"); + SchemaEntity schema1 = + TestUtil.getTestSchemaEntity(2L, "schema1", Namespace.of("metalake", "catalog1"), "cmt"); + TableEntity table1 = + TestUtil.getTestTableEntity(3L, "table1", Namespace.of("metalake", "catalog1", "schema1")); + SchemaEntity schema2 = + TestUtil.getTestSchemaEntity(4L, "schema2", Namespace.of("metalake", "catalog1"), "cmt"); + cache.put(catalog); + cache.put(schema1); + cache.put(table1); + cache.put(schema2); + Assertions.assertEquals(4, cache.size()); + + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + listener.onEntityChange( + List.of(record(EntityType.SCHEMA, schema1.nameIdentifier().toString(), OperateType.DROP))); + + // schema1 and its child table1 are dropped through the forward prefix scan ... + Assertions.assertFalse(cache.contains(schema1.nameIdentifier(), EntityType.SCHEMA)); + Assertions.assertFalse(cache.contains(table1.nameIdentifier(), EntityType.TABLE)); + // ... while the sibling schema2 and the parent catalog stay cached. + Assertions.assertTrue(cache.contains(schema2.nameIdentifier(), EntityType.SCHEMA)); + Assertions.assertTrue(cache.contains(catalog.nameIdentifier(), EntityType.CATALOG)); + } + + @Test + void testAlterInvalidatesTagKey() { + CaffeineEntityCache cache = new CaffeineEntityCache(new Config() {}); + TagEntity tag = TestUtil.getTestTagEntity(1L, "tag1", Namespace.of("m1"), "comment"); + cache.put(tag); + + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + listener.onEntityChange( + List.of(record(EntityType.TAG, tag.nameIdentifier().toString(), OperateType.ALTER))); + + Assertions.assertFalse(cache.contains(tag.nameIdentifier(), EntityType.TAG)); + } + + @Test + void testReplaysEveryCacheableTypeAsInvalidate() { + EntityCache cache = mock(EntityCache.class); + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + + listener.onEntityChange( + List.of( + record(EntityType.CATALOG, "m1.cat1", OperateType.ALTER), + record(EntityType.SCHEMA, "m1.cat1.sch1", OperateType.DROP), + record(EntityType.TABLE, "m1.cat1.sch1.tbl1", OperateType.DROP), + record(EntityType.TAG, "m1.system.tag.tag1", OperateType.ALTER), + record(EntityType.POLICY, "m1.system.policy.p1", OperateType.DROP), + record(EntityType.JOB, "m1.system.job.job-1", OperateType.ALTER))); + + verify(cache).invalidate(NameIdentifier.of("m1", "cat1"), EntityType.CATALOG); + verify(cache).invalidate(NameIdentifier.of("m1", "cat1", "sch1"), EntityType.SCHEMA); + verify(cache).invalidate(NameIdentifier.of("m1", "cat1", "sch1", "tbl1"), EntityType.TABLE); + verify(cache).invalidate(NameIdentifier.of("m1", "system", "tag", "tag1"), EntityType.TAG); + verify(cache).invalidate(NameIdentifier.of("m1", "system", "policy", "p1"), EntityType.POLICY); + verify(cache).invalidate(NameIdentifier.of("m1", "system", "job", "job-1"), EntityType.JOB); + } + + @Test + void testSkipsMalformedRowsAndContinues() { + EntityCache cache = mock(EntityCache.class); + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + + listener.onEntityChange( + List.of( + record(null, "m1.cat1", OperateType.ALTER), + record(EntityType.CATALOG, null, OperateType.ALTER), + new EntityChangeRecord(1L, "m1", "NOT_A_TYPE", "m1.x", OperateType.ALTER, 0L), + record(EntityType.CATALOG, "m1.cat_ok", OperateType.ALTER))); + + // Only the single well-formed row is applied. + verify(cache, times(1)).invalidate(any(NameIdentifier.class), any(EntityType.class)); + verify(cache).invalidate(NameIdentifier.of("m1", "cat_ok"), EntityType.CATALOG); + } + + @Test + void testOneFailingRowDoesNotStopTheRest() { + EntityCache cache = mock(EntityCache.class); + NameIdentifier failing = NameIdentifier.of("m1", "boom"); + doThrow(new RuntimeException("boom")).when(cache).invalidate(failing, EntityType.CATALOG); + + EntityCacheChangeLogListener listener = new EntityCacheChangeLogListener(cache); + listener.onEntityChange( + List.of( + record(EntityType.CATALOG, "m1.boom", OperateType.DROP), + record(EntityType.CATALOG, "m1.ok", OperateType.DROP))); + + verify(cache).invalidate(NameIdentifier.of("m1", "ok"), EntityType.CATALOG); + } + + @Test + void testCaffeineAndNoOpsCachesAreLocalPerNode() { + Assertions.assertEquals( + Coherence.LOCAL_PER_NODE, new CaffeineEntityCache(new Config() {}).coherence()); + Assertions.assertEquals(Coherence.LOCAL_PER_NODE, new NoOpsCache(new Config() {}).coherence()); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheCrossNodeInvalidation.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheCrossNodeInvalidation.java new file mode 100644 index 00000000000..ca3fca7e97d --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestEntityCacheCrossNodeInvalidation.java @@ -0,0 +1,122 @@ +/* + * 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.storage.relational; + +import java.io.IOException; +import org.apache.gravitino.Config; +import org.apache.gravitino.Entity.EntityType; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.cache.CaffeineEntityCache; +import org.apache.gravitino.meta.SchemaEntity; +import org.apache.gravitino.meta.TableEntity; +import org.apache.gravitino.storage.RandomIdGenerator; +import org.apache.gravitino.storage.relational.service.SchemaMetaService; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.TestTemplate; + +/** + * End-to-end test of the caffeine cross-node invalidation chain: a write on one node emits an + * {@code entity_change_log} row, and a second node's poller replays it into that node's own {@link + * CaffeineEntityCache}. "Node A" is the shared {@link #backend}; "node B" is a separate cache kept + * fresh by an {@link EntityCacheChangeLogListener} on its own {@link EntityChangeLogPoller}. + */ +public class TestEntityCacheCrossNodeInvalidation extends TestJDBCBackend { + + private static final String METALAKE = "metalake_cross_node"; + private static final String CATALOG = "catalog_cross_node"; + private static final String SCHEMA = "schema_cross_node"; + + // A large poll interval so the background scheduler never fires during the test; the test drives + // node B's poll explicitly via pollChanges(). + private EntityChangeLogPoller newIdlePoller(CaffeineEntityCache nodeBCache) { + EntityChangeLogPoller poller = new EntityChangeLogPoller(3600); + poller.registerListener(new EntityCacheChangeLogListener(nodeBCache)); + // start() seeds the cursor with the current DB tail, modelling a node whose cache is already + // warm: only changes written after this point are replayed. + poller.start(); + return poller; + } + + @TestTemplate + void testAlterOnNodeAIsReflectedOnNodeBAfterOnePoll() throws IOException { + createParentEntities(METALAKE, CATALOG, SCHEMA, AUDIT_INFO); + Namespace namespace = Namespace.of(METALAKE, CATALOG, SCHEMA); + TableEntity table = + createTableEntity(RandomIdGenerator.INSTANCE.nextId(), namespace, "table1", AUDIT_INFO); + backend.insert(table, false); + + // Node B has already read and cached the table. + CaffeineEntityCache nodeBCache = new CaffeineEntityCache(new Config() {}); + nodeBCache.put(table); + Assertions.assertTrue(nodeBCache.contains(table.nameIdentifier(), EntityType.TABLE)); + + EntityChangeLogPoller nodeBPoller = newIdlePoller(nodeBCache); + try { + // Node A renames the table, which emits an ALTER row in the shared change log. + backend.update( + table.nameIdentifier(), + EntityType.TABLE, + entity -> createTableEntity(table.id(), namespace, "table2", AUDIT_INFO)); + + // Before node B polls, it still serves the stale copy. + Assertions.assertTrue(nodeBCache.contains(table.nameIdentifier(), EntityType.TABLE)); + + // One poll later, node B has dropped the stale key. + nodeBPoller.pollChanges(); + Assertions.assertFalse(nodeBCache.contains(table.nameIdentifier(), EntityType.TABLE)); + } finally { + nodeBPoller.close(); + } + } + + @TestTemplate + void testSchemaDropOnNodeAClearsChildTablesOnNodeB() throws IOException { + createParentEntities(METALAKE, CATALOG, SCHEMA, AUDIT_INFO); + Namespace tableNamespace = Namespace.of(METALAKE, CATALOG, SCHEMA); + SchemaEntity schema = + SchemaMetaService.getInstance() + .getSchemaByIdentifier(NameIdentifierUtil.ofSchema(METALAKE, CATALOG, SCHEMA)); + TableEntity table = + createTableEntity( + RandomIdGenerator.INSTANCE.nextId(), tableNamespace, "table1", AUDIT_INFO); + backend.insert(table, false); + + // Node B has cached both the schema and its child table. + CaffeineEntityCache nodeBCache = new CaffeineEntityCache(new Config() {}); + nodeBCache.put(schema); + nodeBCache.put(table); + Assertions.assertEquals(2, nodeBCache.size()); + + EntityChangeLogPoller nodeBPoller = newIdlePoller(nodeBCache); + try { + // Node A drops the schema with cascade, emitting a single SCHEMA DROP row. + Assertions.assertTrue( + SchemaMetaService.getInstance().deleteSchema(schema.nameIdentifier(), true)); + + nodeBPoller.pollChanges(); + + // The schema drop cascades to its cached child table on node B via the forward prefix scan. + Assertions.assertFalse(nodeBCache.contains(schema.nameIdentifier(), EntityType.SCHEMA)); + Assertions.assertFalse(nodeBCache.contains(table.nameIdentifier(), EntityType.TABLE)); + } finally { + nodeBPoller.close(); + } + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestRelationalEntityStore.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestRelationalEntityStore.java index 042c29681b8..9fa0126ef35 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/TestRelationalEntityStore.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestRelationalEntityStore.java @@ -108,16 +108,8 @@ void testInsertRelationInvalidatesCacheAfterBackendInsert() Mockito.doAnswer( invocation -> { - Mockito.verify(cache, Mockito.never()) - .invalidate( - src, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - Mockito.verify(cache, Mockito.never()) - .invalidate( - dst, - Entity.EntityType.TAG, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); + Mockito.verify(cache, Mockito.never()).invalidate(src, Entity.EntityType.TABLE); + Mockito.verify(cache, Mockito.never()).invalidate(dst, Entity.EntityType.TAG); return null; }) .when(backend) @@ -147,14 +139,8 @@ void testInsertRelationInvalidatesCacheAfterBackendInsert() dst, Entity.EntityType.TAG, true); - inOrder - .verify(cache) - .invalidate( - src, Entity.EntityType.TABLE, SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - inOrder - .verify(cache) - .invalidate( - dst, Entity.EntityType.TAG, SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); + inOrder.verify(cache).invalidate(src, Entity.EntityType.TABLE); + inOrder.verify(cache).invalidate(dst, Entity.EntityType.TAG); } @Test @@ -170,21 +156,10 @@ void testUpdateEntityRelationsInvalidatesCacheAfterBackendUpdate() Mockito.doAnswer( invocation -> { + Mockito.verify(cache, Mockito.never()).invalidate(src, Entity.EntityType.TABLE); + Mockito.verify(cache, Mockito.never()).invalidate(destToAdd, Entity.EntityType.TABLE); Mockito.verify(cache, Mockito.never()) - .invalidate( - src, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - Mockito.verify(cache, Mockito.never()) - .invalidate( - destToAdd, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - Mockito.verify(cache, Mockito.never()) - .invalidate( - destToRemove, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); + .invalidate(destToRemove, Entity.EntityType.TABLE); return List.of(); }) .when(backend) @@ -211,21 +186,8 @@ void testUpdateEntityRelationsInvalidatesCacheAfterBackendUpdate() Entity.EntityType.TABLE, destEntitiesToAdd, destEntitiesToRemove); - inOrder - .verify(cache) - .invalidate( - src, Entity.EntityType.TABLE, SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - inOrder - .verify(cache) - .invalidate( - destToAdd, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); - inOrder - .verify(cache) - .invalidate( - destToRemove, - Entity.EntityType.TABLE, - SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL); + inOrder.verify(cache).invalidate(src, Entity.EntityType.TABLE); + inOrder.verify(cache).invalidate(destToAdd, Entity.EntityType.TABLE); + inOrder.verify(cache).invalidate(destToRemove, Entity.EntityType.TABLE); } } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestEntityChangeLogService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestEntityChangeLogService.java index f58c6b9da32..1b2390893f9 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestEntityChangeLogService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestEntityChangeLogService.java @@ -18,20 +18,31 @@ */ package org.apache.gravitino.storage.relational.service; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import java.io.IOException; import java.util.List; import java.util.Map; import org.apache.gravitino.Catalog; import org.apache.gravitino.Entity; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.Namespace; +import org.apache.gravitino.job.JobHandle; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.FilesetEntity; +import org.apache.gravitino.meta.JobEntity; +import org.apache.gravitino.meta.JobTemplateEntity; import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.PolicyEntity; import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.meta.TableEntity; +import org.apache.gravitino.meta.TagEntity; import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.meta.ViewEntity; +import org.apache.gravitino.policy.Policy; +import org.apache.gravitino.policy.PolicyContent; +import org.apache.gravitino.policy.PolicyContents; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; @@ -362,4 +373,127 @@ void testLeafEntityChangeLogOnRenameAndDrop() throws IOException { NameIdentifierUtil.ofModel(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, "model2").toString(), OperateType.DROP); } + + @TestTemplate + void testTagChangeLogOnAlterAndDrop() throws IOException { + createAndInsertMakeLake(METALAKE_NAME); + TagEntity tag = createAndInsertTagEntity("tag1", "tag comment", METALAKE_NAME); + + long maxIdBeforeTagAlter = maxEntityChangeId(); + TagMetaService.getInstance() + .updateTag( + tag.nameIdentifier(), + entity -> + TagEntity.builder() + .withId(tag.id()) + .withName(tag.name()) + .withNamespace(tag.namespace()) + .withComment("tag comment updated") + .withProperties(ImmutableMap.of()) + .withAuditInfo(AUDIT_INFO) + .build()); + assertEntityChange( + maxIdBeforeTagAlter, + METALAKE_NAME, + Entity.EntityType.TAG, + NameIdentifierUtil.ofTag(METALAKE_NAME, "tag1").toString(), + OperateType.ALTER); + + long maxIdBeforeTagDrop = maxEntityChangeId(); + Assertions.assertTrue(TagMetaService.getInstance().deleteTag(tag.nameIdentifier())); + assertEntityChange( + maxIdBeforeTagDrop, + METALAKE_NAME, + Entity.EntityType.TAG, + NameIdentifierUtil.ofTag(METALAKE_NAME, "tag1").toString(), + OperateType.DROP); + } + + @TestTemplate + void testPolicyChangeLogOnAlterAndDrop() throws IOException { + createAndInsertMakeLake(METALAKE_NAME); + PolicyContent content = + PolicyContents.custom( + ImmutableMap.of("field1", 1), ImmutableSet.of(MetadataObject.Type.TABLE), null); + PolicyEntity policy = + createAndInsertPolicyEntity("policy1", "policy comment", content, METALAKE_NAME); + + long maxIdBeforePolicyAlter = maxEntityChangeId(); + PolicyMetaService.getInstance() + .updatePolicy( + policy.nameIdentifier(), + entity -> + PolicyEntity.builder() + .withId(policy.id()) + .withName(policy.name()) + .withNamespace(policy.namespace()) + .withPolicyType(Policy.BuiltInType.CUSTOM) + .withComment("policy comment updated") + .withEnabled(true) + .withContent(content) + .withAuditInfo(AUDIT_INFO) + .build()); + assertEntityChange( + maxIdBeforePolicyAlter, + METALAKE_NAME, + Entity.EntityType.POLICY, + NameIdentifierUtil.ofPolicy(METALAKE_NAME, "policy1").toString(), + OperateType.ALTER); + + long maxIdBeforePolicyDrop = maxEntityChangeId(); + Assertions.assertTrue(PolicyMetaService.getInstance().deletePolicy(policy.nameIdentifier())); + assertEntityChange( + maxIdBeforePolicyDrop, + METALAKE_NAME, + Entity.EntityType.POLICY, + NameIdentifierUtil.ofPolicy(METALAKE_NAME, "policy1").toString(), + OperateType.DROP); + } + + @TestTemplate + void testJobChangeLogOnOverwriteAndDrop() throws IOException { + createAndInsertMakeLake(METALAKE_NAME); + JobTemplateEntity jobTemplate = + TestJobTemplateMetaService.newShellJobTemplateEntity( + "job_template_for_change_log", "comment", METALAKE_NAME); + JobTemplateMetaService.getInstance().insertJobTemplate(jobTemplate, false); + + JobEntity job = + TestJobTemplateMetaService.newJobEntity( + jobTemplate.name(), JobHandle.Status.QUEUED, METALAKE_NAME); + + // A plain insert (create) must not emit a change-log row: list bypasses the cache and there is + // no negative caching. + long maxIdBeforeCreate = maxEntityChangeId(); + JobMetaService.getInstance().insertJob(job, false); + Assertions.assertEquals(maxIdBeforeCreate, maxEntityChangeId()); + + // An overwrite is an in-place status update, so it emits an ALTER row. + long maxIdBeforeOverwrite = maxEntityChangeId(); + JobEntity runningJob = + JobEntity.builder() + .withId(job.id()) + .withJobExecutionId(job.jobExecutionId()) + .withNamespace(job.namespace()) + .withJobTemplateName(job.jobTemplateName()) + .withStatus(JobHandle.Status.STARTED) + .withAuditInfo(AUDIT_INFO) + .build(); + JobMetaService.getInstance().insertJob(runningJob, true); + assertEntityChange( + maxIdBeforeOverwrite, + METALAKE_NAME, + Entity.EntityType.JOB, + job.nameIdentifier().toString(), + OperateType.ALTER); + + long maxIdBeforeJobDrop = maxEntityChangeId(); + Assertions.assertTrue(JobMetaService.getInstance().deleteJob(job.nameIdentifier())); + assertEntityChange( + maxIdBeforeJobDrop, + METALAKE_NAME, + Entity.EntityType.JOB, + job.nameIdentifier().toString(), + OperateType.DROP); + } }