[
https://issues.apache.org/jira/browse/IGNITE-23834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Roman Puchkovskiy updated IGNITE-23834:
---------------------------------------
Description:
All non-test implementations of RaftGroupListener behave identically with
respect to error handling. They have code like
} catch (IgniteInternalException e) {
clo.result(e);
} catch (CompletionException e) {
clo.result(e.getCause());
} ...
Calling clo.result() with an exception is not enough to let JRaft know that the
state machine might be broken; this just causes the error being logged and the
exception being sent to the client (if this is the leader).
As JRaft doesn't know that an error happened, it keeps applying subsequent
commands. If it so happens that command with index 10 is applied on node A, but
on B it throws an IgniteInternalException (and most of our exceptions are
subclasses of this class) or a CompletionException (which might wrap absolutely
anything), then a command with index 11 might still be applied on both nodes.
This would result in state machines' states diverging.
We should rethrow any throwable we catch by default to allow JRaft know about
it (if it sees such a throwable, it will mark the state machine as broken and
stop applying subsequent commands, which seems to be the only safe default
behavior). If, in the future, we realize that some exceptions might be normally
(also predictably based on the state machine state and command, and
idempotently) thrown, we should whitelist them individually. Probably, the best
way would be to create a specific exception class like
StateMachineLogicException. But currently, we don't seem to have such
exceptions at all, and by default we should just rethrow them to JRaft.
Another problem is that
> RaftGroupListener implementations may silently break state machine state
> ------------------------------------------------------------------------
>
> Key: IGNITE-23834
> URL: https://issues.apache.org/jira/browse/IGNITE-23834
> Project: Ignite
> Issue Type: Bug
> Reporter: Roman Puchkovskiy
> Assignee: Roman Puchkovskiy
> Priority: Major
> Labels: ignite-3
>
> All non-test implementations of RaftGroupListener behave identically with
> respect to error handling. They have code like
> } catch (IgniteInternalException e) {
> clo.result(e);
> } catch (CompletionException e) {
> clo.result(e.getCause());
> } ...
> Calling clo.result() with an exception is not enough to let JRaft know that
> the state machine might be broken; this just causes the error being logged
> and the exception being sent to the client (if this is the leader).
> As JRaft doesn't know that an error happened, it keeps applying subsequent
> commands. If it so happens that command with index 10 is applied on node A,
> but on B it throws an IgniteInternalException (and most of our exceptions
> are subclasses of this class) or a CompletionException (which might wrap
> absolutely anything), then a command with index 11 might still be applied on
> both nodes. This would result in state machines' states diverging.
> We should rethrow any throwable we catch by default to allow JRaft know about
> it (if it sees such a throwable, it will mark the state machine as broken and
> stop applying subsequent commands, which seems to be the only safe default
> behavior). If, in the future, we realize that some exceptions might be
> normally (also predictably based on the state machine state and command, and
> idempotently) thrown, we should whitelist them individually. Probably, the
> best way would be to create a specific exception class like
> StateMachineLogicException. But currently, we don't seem to have such
> exceptions at all, and by default we should just rethrow them to JRaft.
> Another problem is that
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
