rpuch commented on code in PR #4876:
URL: https://github.com/apache/ignite-3/pull/4876#discussion_r1880266388
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java:
##########
@@ -350,15 +324,16 @@ public void advanceSafeTime(HybridTimestamp time) {
}
private void
notifyFailureHandlerOnFirstFailureInNotificationChain(Throwable e) {
- if (e instanceof NodeStoppingException) {
- return;
- }
-
if (firedFailureOnChain.compareAndSet(false, true)) {
- LOG.info("Notification chain encountered an error, so no
notifications will be ever fired for subsequent revisions "
- + "until a restart. Notifying the FailureManager");
-
- failureManager.process(new FailureContext(CRITICAL_ERROR, e));
+ if (unwrapCause(e) instanceof NodeStoppingException) {
Review Comment:
I think we still need to notify FH regardless of the exception type. The
fact that we are here means that
1. Some watch might not have been run completely
2. No other watch will ever be notified
so we are already in trouble and the best thing we can do is resort to the
FH.
So it seems that this `if` should be removed.
It's the job of individual watches to make sure that a
`NodeStoppingException` does not get 'thrown' from them. Only the watch itself
knows how to handle such an exception. If the exception escapes to the
infrastructure, it can only fail the node.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
