Copilot commented on code in PR #12445:
URL: https://github.com/apache/gravitino/pull/12445#discussion_r3768107905
##########
core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogPoller.java:
##########
@@ -410,39 +307,29 @@ private List<EntityChangeLogListener>
notifyListeners(BatchDelivery delivery) {
delivery.firstChangeId(),
delivery.lastChangeId);
} catch (Exception e) {
- failedListeners.add(listener);
- LOG.warn(
- "Entity change log listener {} failed to consume batch id range
[{}, {}]",
+ LOG.error(
+ "Entity change log listener {} failed to consume batch id range
[{}, {}]; the batch is "
+ + "not retried, so the listener must have recovered locally",
listener.getClass().getName(),
Review Comment:
The error log says the listener "must have recovered locally" even though
this is the path where the listener threw; in practice this can also be the
case where local recovery failed (e.g., cache clear throws). This wording can
mislead operators when debugging coherence issues; it should instead state that
the listener is responsible for local recovery because the batch will not be
retried.
--
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]
