cluster: set feature_table version during bootstrap #8225
Conversation
src/v/cluster/bootstrap_backend.cc
Outdated
| co_await _feature_table.invoke_on_all( | ||
| [v = cmd.value.founding_version](features::feature_table& ft) { | ||
| ft.set_active_version(v); |
There was a problem hiding this comment.
I think we need to avoid doing this if the version is lower than the current active version, e.g. if we've already loaded a feature table snapshot and are now replaying the controller log that was started on an older version.
There was a problem hiding this comment.
Good catch, you're right. I think we can be even stricter, and just only set the active version if the current active version is invalid_version: this should only be happening when we have a totally default-constructed feature table.
jcsp
force-pushed
the
issue-8203
branch
2 times, most recently
from
d7e1be2
to
4368bf8
Compare
|
This caused a failure in |
jcsp
force-pushed
the
issue-8203
branch
2 times, most recently
from
5d71e97
to
a3de4b1
Compare
The "normal" set_active_version method increments the version but does not activate features (though it may make them available). This is because activating features on upgrade is optional, and that optionality is respected at time of writing to the controller log, not replaying it. The reason for making auto-activation optional is to enable cautious upgrades, but during cluster bootstrap this motivation goes away, so we will unconditionally switch on all the available features when we see the bootstrap message's cluster version in the controller log. That is what this function does, as well as calling through to set_active_version.
This enables nodes reading the record to fast-forward their feature table to this version, without waiting for the feature_manager logic to see all nodes' versions and activate that way. Usually this is superfluous, when a founding triplet of nodes will be up promptly, but in some cases we might run with just two nodes for some time, where these two nodes would otherwise wait for the third to report its version before advancing the feature table version.
It will use this for setting the cluster logical version proactively when it sees one in a bootstrap message.
In some circumstances, a cluster can have two nodes up+bootstrapped, with an active controller raft group, but a third founding node trying to join the cluster by requesting an ID (because this node reported in earlier, but had trouble following through with the bootstrap process). Because handing out node IDs is behind a feature gate, this would fail, and the cluster logical version would not advance because there was a node in the members_table who had not reported its logical version. Avoid this class of issues by including logical version in the controller bootstrap event, so that as soon as there is a bootstrap message in the log, we are certain to have activated all the features that a node might need to proceed. Fixes redpanda-data#8203
|
This one snowballed a bit... I realized that bootstrap_backend also needs to write a snapshot, because once it has applied the latest version, there is no subsequent feature_manager-driven update that would prompt a snapshot. While looking at this, I was also reminded that we go through this awkward phase pre-cluster-join where we have no logical cluster version. I've added a commit that fast-forwards the cluster version prior to joining the cluster. This is kind of orthogonal to the issue originally fixed in this PR, but made sense while I had my mind on it. One thing I haven't done here is to make that node join totally safe by refusing to let newer-version nodes join the cluster (because they would have set their own feature_table to a higher one than the cluster is using). Currently we validate the version of joining nodes, but only to refuse to let too-old nodes join. Maybe some discussion needed on how to make this check, or whether we should instead try to tolerate newer nodes and teach them to rewind their feature table when they join an older version cluster. |
|
The early population of cluster version prior to node join was getting hairy, so I split it off in #8282 |
|
/ci-repeat 5 |
|
1 failure in those repeats, which was: |
|
/backport v22.3.x |
|
Failed to run cherry-pick command. I executed the below command: |
[v22.3.x] Backport #8225 (cluster: set feature_table version during bootstrap)
Fixes #8203
Backports Required
UX Changes
None
Release Notes
Bug Fixes