-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply future deadlocks if quorum cannot be reached #498
Comments
Hey there, |
Please keep this open. |
Right now (February 2025) the issue still persists. Found that yesterday on the latest version of the library. Error case: used Apply future with Error() blocking call, then if cluster is changed while the action (for example, leader changed), error doesn’t return even after ApplyTimeout (but seems like it should be something like ErrLeaderChanged) and the function deadlocks never returning the future. |
I believe this is a duplicate of #145 which was closed due to no activity but appears to be unresolved.
I have calls to
Apply
which sometimes occur at the same time as nodes in the cluster being stopped, e.g. in unit/integration tests. In this situation, waiting on the result from the future returned byApply
(deferError.Error()
) will block forever, causing the test to deadlock. This is becausedeferError
is waiting on a response from theerrCh
which is never going to come.Interestingly, a change was added a while back to add a
ShutdownCh
todeferError
to allow futures to react to shutdowns. However, this appears to only be used to address a deadlock that occurred when snapshots are taken.It seems to me like
deferError
ought to have a timeout on it, either taking its own timeout on calls toError()
or by respecting the timeout passed toApply()
. Given that the future returned byApply
involves RPCs and consensus, it seems like this should have a timeout to prevent potential deadlock situations.The text was updated successfully, but these errors were encountered: