Infrastructure for assuming cluster features in the next major version #118143
Conversation
thecoop added >feature :Core/Infra/Core 
| Hi @thecoop, I've created a changelog YAML for you. |
thecoop force-pushed the cluster-feature-majors branch from 654b6de to 885e8de Compare | This uses |
thecoop force-pushed the cluster-feature-majors branch from cee97fa to b604511 Compare 
elasticsearchmachine added Team:Core/Infra | 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
thecoop force-pushed the cluster-feature-majors branch from 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
| * and so should be assumed to be true for all nodes after that boundary. | ||
| */ | ||
| 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...
elasticsearchmachine added the serverless-linked 
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
mosche left a comment
mosche left a comment There was a problem hiding this comment.
A few more nits / suggestions on naming, but LGTM 👍
| * @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.
| * and so should be assumed to be true for all nodes after that boundary. | ||
| */ | ||
| 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.
5 participants
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: