yuqi1129 opened a new issue, #12440:
URL: https://github.com/apache/gravitino/issues/12440

   ### What would you like to be improved?
   
   `EntityChangeLogPoller` currently pauses its cursor when a listener throws, 
retries the same batch up to `gravitino.entityChangeLog.listenerMaxRetries` 
(default 10) times, and then applies 
`gravitino.entityChangeLog.listenerFailureAction` — `EXIT` by default, which 
calls `System.exit(1)` and stops the server.
   
   After #12374, both registered listeners are self-healing and never propagate 
a failure in practice:
   
   - `EntityCacheChangeLogListener` clears its whole cache when an invalidation 
fails. The clear is derived-state recovery: it is a strict superset of the 
invalidation that failed and of the rest of the batch, so no stale entry can 
survive.
   - `CatalogChangeLogListener` already swallows its own failures, as the 
poller Javadoc notes.
   
   That leaves the retry/`EXIT` path 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.
   
   Both configs are `VERSION_2_0_0` and 2.0.0 is unreleased, so they can be 
removed without a deprecation cycle.
   
   ### How should we improve?
   
   Make "dispatch once, always advance the cursor, listeners are self-healing" 
the contract, and let each listener recover locally:
   
   - Remove `gravitino.entityChangeLog.listenerMaxRetries` and 
`gravitino.entityChangeLog.listenerFailureAction` from `Configs`, their wiring 
in `RelationalEntityStore`, and their entries in 
`docs/gravitino-server-config.md`.
   - Remove `ListenerFailureAction`, `exitHandler`, `pendingDelivery`, 
`BatchDelivery.retryOnly`/`attempts` and `handleExhaustedRetries` from 
`EntityChangeLogPoller`; keep logging each listener failure at `ERROR`.
   - State the self-healing contract in the poller Javadoc, so a future 
listener that cannot contain its own failures is not added silently.
   - Keep `CatalogChangeLogListener` swallowing its failures — it must **not** 
adopt the cache-clear fallback, because clearing the catalog cache closes 
in-use `IsolatedClassLoader`s (the failure mode behind #11739).
   - Clean up the config stubs in `TestFilesetCatalogOperations`, 
`TestKafkaCatalogOperations`, `TestGenericCatalogOperations`, 
`TestModelCatalogOperations` and `AbstractEntityStorageBenchmark`, and replace 
the retry/EXIT cases in `TestEntityChangeLogPoller` with one asserting that a 
throwing listener neither pauses the cursor nor blocks other listeners.
   


-- 
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]

Reply via email to