[#11232][#11233] improvement(core): Global EntityChangeLogPoller with CatalogManager cache invalidation and automatic retention cleanup#11254
Open
yuqi1129 wants to merge 11 commits into
Conversation
…gPoller with CatalogManager cache invalidation and automatic retention cleanup Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes entity_change_log consumption into a single process-wide poller in core, adds listener-driven cache invalidation for CatalogManager in HA deployments, and introduces periodic retention cleanup so entity_change_log doesn’t grow unbounded.
Changes:
- Introduces a global
EntityChangeLogPollerwith listener registration and periodic retention pruning. - Adds
CatalogChangeLogListenerand wires it (andJcasbinChangePoller) intoGravitinoEnv/JcasbinAuthorizerfor cross-node cache invalidation. - Extends catalog update flow to write change-log entries on all catalog updates (not only renames) and adds new configuration keys for poll/retention/cleanup intervals.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java | Updates tests to drive entity-change handling via listener callback rather than DB polling. |
| server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangePoller.java | Converts entity-change handling into an EntityChangeLogListener callback; keeps owner polling local. |
| server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java | Registers/unregisters the authorizer’s poller as a global change-log listener when available. |
| server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java | Refactors owner lookup loader path and alters negative-caching behavior for missing owners. |
| core/src/test/java/org/apache/gravitino/storage/relational/TestEntityChangeLogPoller.java | Adds unit tests for global poller dispatch, cursor advancement, and retention pruning gating. |
| core/src/test/java/org/apache/gravitino/storage/relational/service/TestEntityChangeLogService.java | Extends service tests to verify ALTER (non-rename) emits an entity change log record. |
| core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java | Adds a unit test asserting catalog cache invalidation on consumed catalog change records. |
| core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java | Writes catalog change-log entries on any successful catalog update (not only rename). |
| core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogPoller.java | New global poller implementation with listener dispatch and periodic retention cleanup. |
| core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogListener.java | New listener interface for batches consumed from entity_change_log. |
| core/src/main/java/org/apache/gravitino/GravitinoEnv.java | Creates/starts/stops the global poller for relational stores and registers catalog listener. |
| core/src/main/java/org/apache/gravitino/Configs.java | Adds new configuration keys and defaults for poll interval, retention, and cleanup interval. |
| core/src/main/java/org/apache/gravitino/catalog/CatalogChangeLogListener.java | New listener that invalidates CatalogManager’s local catalog cache on catalog changes. |
…alogChangeLogListener When CatalogManager alters/drops a catalog locally, it already updates the cache correctly. The EntityChangeLogPoller would later pick up the same change and redundantly invalidate the fresh cache entry, closing the IsolatedClassLoader and causing NoClassDefFoundError on the next operation. Track local mutation counts per catalog identifier so the listener can distinguish local changes (skip) from remote HA changes (invalidate). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Code Coverage Report
Files
|
Resolve conflicts in JcasbinChangePoller: keep EntityChangeLogListener approach from this PR (entity changes dispatched by global EntityChangeLogPoller) while adopting the keyset (updated_at, id) owner cursor from main/apache#11088 which correctly tracks soft-deletes alongside inserts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n TestJcasbinAuthorizationLookups Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ITINO_HOME
The gravitino.sh.template was updated to include \${GRAVITINO_HOME} in the
PID lookup. The multi-instance test's patch_pid_grep function was asserting the
old pattern still existed and failing. Skip the patch when the script already
contains the home-scoped filter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
loadOwner now returns Optional.empty() instead of throwing NoSuchOwnerException, so Caffeine can store the negative result. Subsequent requests with a fresh AuthorizationRequestContext find the absent owner in the shared cache and skip the DB query entirely. Also update the same-context test: putCount is now 1 (absent result IS stored in the shared cache on first load) and rename the test to reflect the new behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ce88587 to
e333894
Compare
e333894 to
9c66d11
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
EntityChangeLogPollerincorewith a listener-based dispatch pattern, replacing the per-consumer polling inJcasbinChangePoller.CatalogChangeLogListenerthat invalidatesCatalogManager's local catalog cache when catalog ALTER/RENAME/DROP changes are consumed, providing eventual consistency across HA nodes.entity_change_log— expired rows are pruned periodically (default: 1-hour interval, 1-day retention), with configurable settings.Why are the changes needed?
In a multi-node deployment, catalog changes on one server do not invalidate the
CatalogManagercache on peer servers until local TTL eviction. This can leave peers using stale catalog entities or class loaders. Additionally,entity_change_loggrows unboundedly without automatic cleanup.Fix: #11232
Fix: #11233
Does this PR introduce any user-facing change?
Yes — three new configuration keys:
gravitino.entityChangeLog.pollIntervalSecs(default 3)gravitino.entityChangeLog.retentionSecs(default 86400)gravitino.entityChangeLog.cleanupIntervalSecs(default 3600)How was this patch tested?
TestEntityChangeLogPoller— unit tests for dispatch, cursor advancement, listener failure semantics, retention pruning, and cleanup interval gating.TestCatalogManager.testCatalogChangeLogListenerInvalidatesCatalogCache— verifies cache invalidation on entity change.TestEntityChangeLogService— extended to cover ALTER (non-rename) change log writing.TestJcasbinChangePoller— updated for listener-based entity change delivery.