-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[8.x] Allow archive and searchable snapshots indices in N-2 version #118923
Conversation
ba74897
to
8ecac81
Compare
Relates ES-10274
8ecac81
to
a637679
Compare
@@ -124,7 +124,7 @@ public static IndexVersion current() { | |||
} | |||
|
|||
public boolean isLegacyIndexVersion() { | |||
return before(IndexVersions.MINIMUM_COMPATIBLE); | |||
return before(IndexVersions.MINIMUM_READONLY_COMPATIBLE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 8.18, MINIMUM_COMPATIBLE is equal to MINIMUM_READONLY_COMPATIBLE as there is no N-2 support, but I changed it here to make it uniform with 9.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder about this. Why do we need MINIMUM_READONLY_COMPATIBLE in 8.x if it's the same as MINIMUM_COMPATIBLE? It seems like the code in 8.x could be much simpler, although it would diverge from main? Anything I am missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No you're not missing anything. Keeping the code similar on both branches helps for backporting changes and I suspect we will have more things to backport in a short future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, up to you!
@@ -1327,12 +1328,16 @@ public ClusterState execute(ClusterState currentState) { | |||
request.indexSettings(), | |||
request.ignoreIndexSettings() | |||
); | |||
if (snapshotIndexMetadata.getCompatibilityVersion().before(minIndexCompatibilityVersion)) { | |||
if (snapshotIndexMetadata.getCompatibilityVersion().isLegacyIndexVersion()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, it does not have an impact on 8.x but makes the code look similar to main.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I think this can go in independently of the 9.0 change?
.settings( | ||
Settings.builder() | ||
.put(INDEX_STORE_TYPE_SETTING.getKey(), SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_STORE_TYPE) | ||
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersions.MINIMUM_READONLY_COMPATIBLE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got fooled here, I wonder if you should comment (or assert) that readonly==regular compatibility to explain to reader?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assertion added
Thanks for the quick review!
I think so, if we are OK leaving a time window (until the 9.0 change is merged) during which indices can be accepted without being really supported on 9.0. |
Ah, right, let us hold off on it then until both are ready - unless you need to build on top of it? |
Thanks Henning & Luca! |
This change (along with the required change #118923 for 8.18) relaxes the index compatibility version checks to allow archive and searchable snapshot indices in version N-2 to exist on a 9.x cluster. Relates ES-10274
This is the backport of #118941 for 8.18.
This change relaxes the index compatibility version checks to allow archive and searchable snapshot indices in version N-2 to exist on a 9.x cluster. It uses the min. read-only index compatible version added in #118884 to accept join requests from 9.x nodes that can read indices created in version 7.x (N-2) as long as they have a write block and are either archive or searchable snapshot indices.
Relates ES-10274