[#10414] improvement(core): Unify cache invalidation in CatalogManager/Jcasbin/EntityStore by yuqi1129 · Pull Request #10416 · apache/gravitino

yuqi1129 · 2026-03-12T12:41:29Z

What changes were made?

This PR implements Phase-1 cache invalidation unification for #10414.

Added a shared cache invalidation framework under core:
- CacheDomain, CacheInvalidationEvent, CacheInvalidationOperation
- CacheInvalidationHandler, CacheInvalidationService
- local in-JVM dispatcher implementation and factory
- typed invalidation keys for entity/catalog/role/owner caches
Added config toggle:
- gravitino.cache.invalidation.enabled (default: true)
Wired unified invalidation into:
- RelationalEntityStore (entity/relation mutations including batchPut and batchDelete)
- CatalogManager (catalog cache invalidation paths)
- JcasbinAuthorizer (loadedRoles and ownerRel invalidation)
Added tests:
- TestLocalCacheInvalidationService

Why are the changes needed?

Cache invalidation logic was previously spread across modules with inconsistent mutation coverage. This change introduces a shared contract and local dispatcher so invalidation behavior is explicit and consistent across key high-risk modules.

Does this PR introduce any user-facing change?

No functional API change. Internal cache invalidation behavior is standardized.

How was this patch tested?

./gradlew :core:spotlessApply :server-common:spotlessApply --no-daemon
./gradlew :core:compileJava :server-common:compileJava :core:test --tests org.apache.gravitino.cache.invalidation.TestLocalCacheInvalidationService :core:test --tests org.apache.gravitino.catalog.TestCatalogManager :server-common:test --tests org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer --no-daemon

Closes #10414

Introduce phase-1 cache invalidation framework with local dispatcher and wire CatalogManager, RelationalEntityStore, and JcasbinAuthorizer to publish/handle unified invalidation events. Add config toggle and unit tests for the local invalidation service. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Introduces a shared, local (in-JVM) cache invalidation framework and wires it into core mutation paths to standardize invalidation behavior across CatalogManager, RelationalEntityStore, and JcasbinAuthorizer.

Changes:

Added core cache invalidation primitives (domains, events, operations, handlers) and a local dispatcher + singleton factory.
Integrated unified invalidation publishing/handling into catalog, entity store, and authorization caches behind a config toggle.
Added unit tests for the local invalidation service.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java	Registers invalidation handlers and publishes role/owner cache invalidations via unified service
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java	Publishes entity/relation invalidation events on mutations; registers entity invalidation handler
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java	Publishes catalog cache invalidation events and handles them via a registered handler
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheDomain.java	Defines invalidation domains
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheInvalidationEvent.java	Defines immutable invalidation event representation
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheInvalidationHandler.java	Introduces handler functional interface
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheInvalidationOperation.java	Defines invalidation operations
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheInvalidationService.java	Defines invalidation service contract
core/src/main/java/org/apache/gravitino/cache/invalidation/CacheInvalidationServiceFactory.java	Provides singleton invalidation service creation
core/src/main/java/org/apache/gravitino/cache/invalidation/LocalCacheInvalidationService.java	Implements local dispatcher and counters
core/src/main/java/org/apache/gravitino/cache/invalidation/CatalogCacheInvalidationKey.java	Adds typed key for catalog cache invalidation
core/src/main/java/org/apache/gravitino/cache/invalidation/EntityCacheInvalidationKey.java	Adds typed key for entity/relation cache invalidation
core/src/main/java/org/apache/gravitino/cache/invalidation/OwnerCacheInvalidationKey.java	Adds typed key for owner cache invalidation
core/src/main/java/org/apache/gravitino/cache/invalidation/RoleCacheInvalidationKey.java	Adds typed key for role cache invalidation
core/src/test/java/org/apache/gravitino/cache/invalidation/TestLocalCacheInvalidationService.java	Adds tests for local publish/disable/unregister behavior
core/src/main/java/org/apache/gravitino/Configs.java	Adds config toggle for cache invalidation enablement

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T12:47:44Z

+      return;
+    }
+
+    if (event.operation() != CacheInvalidationOperation.INVALIDATE_KEY


publishOrApplyEntityUpsert publishes UPSERT_KEY, but the registered handler only processes INVALIDATE_ALL and INVALIDATE_KEY. As a result, put() / batchPut() will not invalidate caches on other nodes (and not even via the handler locally), which can leave stale data. Consider either (a) publishing INVALIDATE_KEY for upserts in Phase-1, or (b) updating handleEntityInvalidationEvent to also process UPSERT_KEY (e.g., treat it as invalidate-by-key unless you plan to distribute values).

Suggested change

if (event.operation() != CacheInvalidationOperation.INVALIDATE_KEY

if ((event.operation() != CacheInvalidationOperation.INVALIDATE_KEY

&& event.operation() != CacheInvalidationOperation.UPSERT_KEY)

Copilot · 2026-03-12T12:47:44Z

+          handler.handle(event);
+          handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();


Handler exceptions will currently propagate out of publish(), which can prevent remaining handlers from running and can break the caller’s mutation path (e.g., entity store update/delete) due to a cache-only side effect. Wrap each handler.handle(event) in a try/catch, log failures (including handlerId/domain/operation), and continue dispatching to the rest to improve resilience.

Suggested change

handler.handle(event);

handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();

try {

handler.handle(event);

handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();

} catch (Exception e) {

LOG.error(

"Cache invalidation handler '{}' failed for domain {} and operation {}",

handlerId,

event.domain(),

event.operation(),

e);

}

Copilot · 2026-03-12T12:47:44Z

+import org.apache.gravitino.Config;
+import org.apache.gravitino.Configs;
+
+/** Singleton factory for cache invalidation service. */
+public final class CacheInvalidationServiceFactory {
+  private static volatile CacheInvalidationService invalidationService;
+
+  private CacheInvalidationServiceFactory() {}
+
+  public static CacheInvalidationService getOrCreate(Config config) {
+    if (invalidationService == null) {
+      synchronized (CacheInvalidationServiceFactory.class) {
+        if (invalidationService == null) {
+          invalidationService =
+              new LocalCacheInvalidationService(config.get(Configs.CACHE_INVALIDATION_ENABLED));
+        }
+      }
+    }
+    return invalidationService;
+  }
+
+  @VisibleForTesting
+  public static void resetForTest() {
+    synchronized (CacheInvalidationServiceFactory.class) {
+      invalidationService = null;


Because this is a process-wide singleton, the enabled value is effectively “latched” based on the first getOrCreate(config) call; later calls with different configs can’t change the behavior. This can lead to surprising behavior across modules/tests or when config initialization order changes. Consider enforcing a single initialization point (e.g., explicit init), or detect/log when subsequent configs disagree, or make the service’s enabled state derived from config rather than constructor-time.

Suggested change

import org.apache.gravitino.Config;

import org.apache.gravitino.Configs;

/** Singleton factory for cache invalidation service. */

public final class CacheInvalidationServiceFactory {

private static volatile CacheInvalidationService invalidationService;

private CacheInvalidationServiceFactory() {}

public static CacheInvalidationService getOrCreate(Config config) {

if (invalidationService == null) {

synchronized (CacheInvalidationServiceFactory.class) {

if (invalidationService == null) {

invalidationService =

new LocalCacheInvalidationService(config.get(Configs.CACHE_INVALIDATION_ENABLED));

}

}

}

return invalidationService;

}

@VisibleForTesting

public static void resetForTest() {

synchronized (CacheInvalidationServiceFactory.class) {

invalidationService = null;

import java.util.logging.Logger;

import org.apache.gravitino.Config;

import org.apache.gravitino.Configs;

/** Singleton factory for cache invalidation service. */

public final class CacheInvalidationServiceFactory {

private static final Logger LOGGER =

Logger.getLogger(CacheInvalidationServiceFactory.class.getName());

private static volatile CacheInvalidationService invalidationService;

/**

* Remembers the initial value of {@link Configs#CACHE_INVALIDATION_ENABLED} used to construct

* the singleton. This allows us to detect and log subsequent calls that use a different value.

*/

private static volatile Boolean initialEnabledConfig;

private CacheInvalidationServiceFactory() {}

public static CacheInvalidationService getOrCreate(Config config) {

boolean enabled = config.get(Configs.CACHE_INVALIDATION_ENABLED);

CacheInvalidationService localService = invalidationService;

if (localService != null) {

Boolean initial = initialEnabledConfig;

if (initial != null && initial.booleanValue() != enabled) {

LOGGER.warning(

"Cache invalidation service already initialized with "

+ "CACHE_INVALIDATION_ENABLED="

+ initial

+ " but getOrCreate was called with a different value: "

+ enabled);

}

return localService;

}

synchronized (CacheInvalidationServiceFactory.class) {

if (invalidationService == null) {

initialEnabledConfig = enabled;

invalidationService = new LocalCacheInvalidationService(enabled);

} else {

Boolean initial = initialEnabledConfig;

if (initial != null && initial.booleanValue() != enabled) {

LOGGER.warning(

"Cache invalidation service already initialized with "

+ "CACHE_INVALIDATION_ENABLED="

+ initial

+ " but getOrCreate was called with a different value: "

+ enabled);

}

}

return invalidationService;

}

}

@VisibleForTesting

public static void resetForTest() {

synchronized (CacheInvalidationServiceFactory.class) {

invalidationService = null;

initialEnabledConfig = null;

Copilot · 2026-03-12T12:47:45Z

+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof CacheInvalidationEvent)) {
+      return false;
+    }
+    CacheInvalidationEvent that = (CacheInvalidationEvent) o;
+    return timestampMillis == that.timestampMillis
+        && domain == that.domain
+        && operation == that.operation
+        && Objects.equals(sourceNode, that.sourceNode)
+        && Objects.equals(key, that.key);
+  }


Including timestampMillis in equals/hashCode makes event equality extremely brittle (two logically identical events won’t compare equal unless created at the same millisecond), which is rarely what callers expect for an “event” value object. If equals/hashCode are needed for tests/collections, consider excluding timestampMillis (and potentially sourceNode) from equality, or avoid overriding equals/hashCode entirely and rely on explicit field assertions where needed.

Copilot · 2026-03-12T12:47:45Z

+      cacheInvalidationService.unregisterHandler(CacheDomain.AUTH_ROLE, roleInvalidationHandlerId);
+      cacheInvalidationService.unregisterHandler(
+          CacheDomain.AUTH_OWNER, ownerInvalidationHandlerId);


unregisterHandler is called with handler IDs that may be null/blank if initialize() didn’t complete (or if construction/initialization order changes). While the current local implementation tolerates blank IDs, the CacheInvalidationService contract doesn’t guarantee that. Guard the unregister calls with non-blank handler IDs (or initialize them eagerly) to avoid shutdown-time exceptions in alternate implementations.

Suggested change

cacheInvalidationService.unregisterHandler(CacheDomain.AUTH_ROLE, roleInvalidationHandlerId);

cacheInvalidationService.unregisterHandler(

CacheDomain.AUTH_OWNER, ownerInvalidationHandlerId);

if (StringUtils.isNotBlank(roleInvalidationHandlerId)) {

cacheInvalidationService.unregisterHandler(

CacheDomain.AUTH_ROLE, roleInvalidationHandlerId);

}

if (StringUtils.isNotBlank(ownerInvalidationHandlerId)) {

cacheInvalidationService.unregisterHandler(

CacheDomain.AUTH_OWNER, ownerInvalidationHandlerId);

}

jerryshao · 2026-03-12T12:48:01Z

We should have a serious design doc and discussion before implementation for this PR.

Copilot AI review requested due to automatic review settings March 12, 2026 12:41

yuqi1129 changed the title ~~[#10414] Unify cache invalidation in CatalogManager/Jcasbin/EntityStore~~ [#10414] improvement(core): Unify cache invalidation in CatalogManager/Jcasbin/EntityStore Mar 12, 2026

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Copilot started reviewing on behalf of yuqi1129 March 12, 2026 12:54 View session

yuqi1129 marked this pull request as draft March 20, 2026 11:16

yuqi1129 closed this Mar 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#10414] improvement(core): Unify cache invalidation in CatalogManager/Jcasbin/EntityStore#10416

[#10414] improvement(core): Unify cache invalidation in CatalogManager/Jcasbin/EntityStore#10416
yuqi1129 wants to merge 1 commit intoapache:mainfrom
yuqi1129:issue_10414

yuqi1129 commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

jerryshao commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (event.operation() != CacheInvalidationOperation.INVALIDATE_KEY
	if ((event.operation() != CacheInvalidationOperation.INVALIDATE_KEY
	&& event.operation() != CacheInvalidationOperation.UPSERT_KEY)

		handler.handle(event);
		handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();

-          handler.handle(event);
-          handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();
+          try {
+            handler.handle(event);
+            handledCounters.computeIfAbsent(counterKey, key -> new AtomicLong()).incrementAndGet();
+          } catch (Exception e) {
+            LOG.error(
+                "Cache invalidation handler '{}' failed for domain {} and operation {}",
+                handlerId,
+                event.domain(),
+                event.operation(),
+                e);
+          }

Conversation

yuqi1129 commented Mar 12, 2026

What changes were made?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

jerryshao commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants