Run a barrier before config changes when required #402
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Like the other barriers PR, this is prompted by looking into the consequences of canonical/raft#302. When we're attempting a config change, we check that no other config change is already in progress (
raft->configuration_uncommitted_index != 0
) -- but in order for that check to work as intended, a leader node has to first make sure its commit index is up to date, and that means running a barrier if no entry from the current term has been appended yet. Concretely, this becomes relevant if a leader "A" replicates and commits a config change, but crashes before propagating the new commit index to any other nodes. The next leader "B" will haveconfiguration_uncommitted_index != 0
, so it can't accept any further config changes until some other entry from its term is committed. To prevent this from inconveniencing dqlite clients, this PR adds a new mode toleader__barrier
, where theneedsBarrier
check is replaced by the condition(See canonical/raft#311 for the implementation of these getters -- this PR won't build unless/until that merges into raft.)
Then we run this version of
leader__barrier
before attempting any config change.This is distinctly WIP -- I'm putting it up now to get feedback on the basic approach, in particular whether we want to handle this in dqlite (as opposed to raft). It will need to be rebased once #395 merges, since I picked up the cursor change from that branch here. If you're reviewing, just ignore the first two commits.