Infrastructure for assuming cluster features in the next major version #118143
Conversation
|
Hi @thecoop, I've created a changelog YAML for you. |
654b6de to
885e8de
Compare
|
This uses |
cee97fa to
b604511
Compare
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
server/src/main/java/org/elasticsearch/cluster/ClusterFeatures.java
Outdated
Show resolved
Hide resolved
3becca3 to
3889d4e
Compare
| } | ||
|
|
||
| private static Version nextMajor() { | ||
| return Version.fromId((Version.CURRENT.major + 1) * 1_000_000 + 99); |
There was a problem hiding this comment.
Until the assert in VersionInformation constructor is removed, we need to continue to use Version here
| */ | ||
| public record NodeFeature(String id) { | ||
| public record NodeFeature(String id, boolean assumedAfterNextCompatibilityBoundary) { |
There was a problem hiding this comment.
Naming might need a bit of work
There was a problem hiding this comment.
I struggle coming up with something better... assumePresenceAfterNextCompatibilityBoundary, but that's even harder to read.... assumedOnNextMajor maybe?
There was a problem hiding this comment.
Though next major is not correct in the context of serverless...
There was a problem hiding this comment.
Yeah, exactly, it was assumedOnNextMajor, but it's always true for serverless, so...
server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java
Show resolved
Hide resolved
| * @see org.elasticsearch.env.BuildVersion#canRemoveAssumedFeatures | ||
| */ | ||
| public static boolean featuresCanBeAssumedForNode(DiscoveryNode node) { | ||
| return node.getBuildVersion().canRemoveAssumedFeatures(); |
There was a problem hiding this comment.
Stumbled over remove in canRemoveAssumedFeatures, that was confusing to me and I kept looking for where we remove those. Maybe turning it around makes it easier to understand: canAssumeRemovedFeatures
There was a problem hiding this comment.
That's not quite right though, its not that all features that are missing can be assumed, it's that the features that this ES has marked as assumed can be taken as being met in a node with this version, regardless of whether they're published by the node or not.
There was a problem hiding this comment.
Yes, you're right 👍
If we can find something better than remove, it'd be great, but certainly not worth delaying this further.
canInferAssumedFeatures, canExpectAssumedFeatures, canSkipAssumedFeatures
Or more targeting the versioning side of this: hasPassedNextCompatiblityBoundary or similar.
| */ | ||
| public record NodeFeature(String id) { | ||
| public record NodeFeature(String id, boolean assumedAfterNextCompatibilityBoundary) { |
There was a problem hiding this comment.
I struggle coming up with something better... assumePresenceAfterNextCompatibilityBoundary, but that's even harder to read.... assumedOnNextMajor maybe?
|
@elasticmachine rerun elasticsearch-ci/part-1 |
💔 Backport failed
You can use sqren/backport to manually backport by running |
elastic#118143) Allow features to be marked as 'assumed', allowing them to be removed in the next major version.
elastic#118143) Allow features to be marked as 'assumed', allowing them to be removed in the next major version.
Add support for node features to be labelled as 'assumed in the next major version', allowing them to be removed.
This is targetted at v9 and v8.18, as the feature removals need to be done on both versions to maintain continued compatibility. Once the infrastructure is merged and backported, the procedure for removing features is as follows: