yuqi1129 opened a new pull request, #12445: URL: https://github.com/apache/gravitino/pull/12445
### What changes were proposed in this pull request? Make "dispatch once, always advance the cursor, listeners are self-healing" the entity change log contract, and let each listener recover locally. Poller and configs: - `EntityChangeLogPoller`: remove `ListenerFailureAction`, `exitHandler`, `pendingDelivery`, `BatchDelivery.retryOnly`/`attempts` and `handleExhaustedRetries`. Each batch is dispatched once and the cursor always advances; every listener failure is logged at `ERROR`. The self-healing contract is stated in the poller and `EntityChangeLogListener` javadoc, so a listener that cannot contain its own failures is not added silently. - Remove `gravitino.entityChangeLog.listenerMaxRetries` and `gravitino.entityChangeLog.listenerFailureAction` from `Configs`, their wiring in `RelationalEntityStore`, and their entries in `docs/gravitino-server-config.md`. - Clean up the removed config stubs in 13 test/benchmark classes. All three registered listeners now recover by clearing the cache they maintain, which is a strict superset of the invalidation that failed and of the rest of the batch: | Listener | Cache | Recovery | |-------------------------------|-------------------|--------------------------------------------------| | `EntityCacheChangeLogListener` | entity cache | clears the whole cache (already did) | | `JcasbinChangeListener` | `metadataIdCache` | clears the whole cache (new) | | `CatalogChangeLogListener` | catalog cache | clears the whole cache (new) | Notes on the two listeners that changed: - `JcasbinChangeListener` is a third change-log listener that the issue did not account for. It tolerated poison rows but propagated a failed invalidation, so under the dispatch-once contract its `metadataIdCache` would feed a stale name→id mapping to authorization decisions until the entry's TTL expired. - `CatalogChangeLogListener` clears the catalog cache on a failed eviction. This is a deliberate tradeoff, documented in its javadoc: clearing closes the `CatalogWrapper` of every cached catalog, including catalogs this process is actively serving, so in-flight requests can hit `NoClassDefFoundError` from a closed `IsolatedClassLoader` (the failure mode of #11739). It is accepted so that a changed catalog is never served stale, and the clear runs only on a failed eviction, off the normal path. Malformed rows and a failed `consumeLocalMutation` probe are still skipped rather than escalated, since they name no eviction to recover. Caches deliberately left alone: `ownerRelCache` is driven by `JcasbinChangeListener`'s own `owner_meta` poller, whose cursor only advances after a successful invalidation batch, so it already retries; `userRoleCache`, `groupRoleCache` and `loadedRoles` are version-validated on every read and cannot go stale from a missed batch. ### Why are the changes needed? After #12374 every registered listener can recover locally, so the retry/`EXIT` path is effectively unreachable while carrying real cost: 1. `EXIT` trades the whole server for a condition a local cache clear already resolves. Killing a node to fix a stale cache entry is a heavy, surprising failure mode for operators. 2. A paused cursor blocks cache invalidation for **every** listener in the process while one listener retries, so a single misbehaving listener degrades cluster-wide coherence for up to 10 poll intervals. 3. The retained batch, `pendingDelivery`, `BatchDelivery.retryOnly`, `attempts` tracking and `handleExhaustedRetries` add machinery and two public configs for a path no listener reaches. Fix: #12440 ### Does this PR introduce _any_ user-facing change? Yes: - Removed config keys `gravitino.entityChangeLog.listenerMaxRetries` and `gravitino.entityChangeLog.listenerFailureAction`. Both are `VERSION_2_0_0` and 2.0.0 is unreleased, so no deprecation cycle is needed. - A node no longer stops itself (`System.exit(1)`) when a listener keeps failing to apply a change log batch. ### How was this patch tested? New and reworked unit tests: - `TestEntityChangeLogPoller`: the four retry/pause/EXIT/SKIP cases are replaced by `testThrowingListenerNeitherPausesCursorNorBlocksOtherListeners` (each batch dispatched exactly once, the healthy listener sees every batch, the cursor advances past both) and `testUnregisteredListenerIsSkipped`. - `TestJcasbinChangePoller` (7 → 14): the `metadataIdCache` clear fallback on prefix, leaf-key and batch-lock failures; a failed clear propagating to the poller; the happy path clearing nothing; `ownerRelCache` not cleared as collateral; plus leaf-vs-prefix keying, which had no coverage. - `TestCatalogChangeLogListener` (3 → 7): the clear on a failed eviction; no clear on the happy path; malformed rows and a failed `consumeLocalMutation` probe skipped without clearing; a failed clear propagating. Suites run locally: `:core:test` (1657 tests) and `:server-common:test` (272 tests) with `--rerun-tasks`, both green, plus the unit tests of the four catalog modules whose config stubs changed. `:core:javadoc` reports no new warnings on the touched files. Docker was not available locally, so docker-tagged tests and integration tests were not run. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
