Archive-Index upgrade compatibility by drempapis · Pull Request #118599 · elastic/elasticsearch

drempapis · 2024-12-12T17:56:07Z

In this pr, archive-indices upgrade compatibility is tested.

The test suite creates a cluster in the N-1 version, where N is the current version. Restores snapshots from old clusters (version 5/6) and upgrades it to the current version. Test methods are executed after each upgrade.

This is based on #118363 and closes ES-10200

…:drempapis/elasticsearch into test/archive-index-version-compatibility

javanna

Thanks @drempapis , I left some comments!

server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifier.java

...rc/javaRestTest/java/org/elasticsearch/lucene/AbstractArchiveIndexCompatibilityTestCase.java

...ompatibility/src/javaRestTest/java/org/elasticsearch/lucene/ArchiveIndexCompatibilityIT.java

drempapis · 2024-12-17T08:49:29Z

...ity/src/javaRestTest/java/org/elasticsearch/oldrepos/archiveindex/RestoreFromVersion5IT.java

+
+import org.elasticsearch.test.cluster.util.Version;
+
+public class RestoreFromVersion5IT extends ArchiveIndexTestCase {


Because the Gradle setup is not flexible, I needed to divide the different base versions into separate classes. For each case, we need a fresh cluster with version Current-1.

drempapis · 2024-12-17T08:51:26Z

...y/src/javaRestTest/java/org/elasticsearch/oldrepos/AbstractUpgradeCompatibilityTestCase.java

+        }
+    }
+
+    public final void verifyArchiveIndexCompatibility(String version) throws Exception {


The Searchable snapshot test will also use this template and implement the abstract recovery method to mount a snapshot.

elasticsearchmachine · 2024-12-17T09:56:27Z

Pinging @elastic/es-search-foundations (Team:Search Foundations)

javanna

Left some small comments, LGTM otherwise

javanna · 2024-12-17T10:49:35Z

...lity/src/javaRestTest/java/org/elasticsearch/oldrepos/archiveindex/ArchiveIndexTestCase.java

+ * Restores snapshots from old-clusters (version 5/6) and upgrades it to the current version.
+ * Test methods are executed after each upgrade.
+ */
+public class ArchiveIndexTestCase extends AbstractUpgradeCompatibilityTestCase {


I think I could use some upgrade mention in this class name as well given it's what it tests? You probably wanted to make a distinction between archive indices restored from snapshot and those mounted as searchable snapshots?

Correct, that was my intention. I considered that the Upgrade functionality was implicitly implied by the parent class, where this one denotes that the test is for ArchiveIndex. The same logic has been implemented for SearchableSnapshot in this pr

javanna · 2024-12-17T10:49:52Z

...lity/src/javaRestTest/java/org/elasticsearch/oldrepos/archiveindex/ArchiveIndexTestCase.java

+ * Test suite for Archive indices backward compatibility with N-2 versions.
+ * The test suite creates a cluster in the N-1 version, where N is the current version.
+ * Restores snapshots from old-clusters (version 5/6) and upgrades it to the current version.
+ * Test methods are executed after each upgrade.


I think it would help to provide an example with concrete versions as well., for clarity

I'll do so, ty!

javanna · 2024-12-17T10:50:48Z

...lity/src/javaRestTest/java/org/elasticsearch/oldrepos/archiveindex/ArchiveIndexTestCase.java

+     * Overrides the snapshot-restore operation for archive-indices scenario.
+     */
+    @Override
+    public void recover(RestClient client, String repository, String snapshot, String index) throws Exception {


I would call this method importArchiveIndex or something along those lines.

Using the template pattern, this template placeholder will be used for the archive-index and searchable-snapshots tests. This method implements a restore operation for archive-index tests and a mount operation for searchable snapshots. Yes, by changing the name to importIndex fits better.

javanna · 2024-12-17T10:51:52Z

@cbuescher would you like to take a look as well?

javanna · 2024-12-17T10:52:54Z

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifier.java

+            && indexMetadata.getCreationVersion().isLegacyIndexVersion()
+            && indexMetadata.getCompatibilityVersion().onOrAfter(IndexVersions.MINIMUM_READONLY_COMPATIBLE)) {
+            return;
+        }


@tlrx are you good with this special case here and the one above that targets archive indices? We are addressing the need to upgrade the compatibility version for archive indices imported first in 8.x, that don't allow to upgrade to 9.x otherwise.

I don't think this is going to work during rolling upgrades, because the master may still be a 8.x node and therefore will reject join requests from 9.x, because a 8.x master will still think that 9.x nodes cannot support indices with a 7.x compatibility version.

I think it does work in the case of a full cluster restart though, since the node will be directly be 9.x with the new logic here.

We have to change that logic to also support searchable snapshot N-2 indices, maybe I can do it for both types of indices at once.

I'm about to merge #118744 that is a prerequisite of #118785 which contains similar logic than the one added here but for searchable snapshots.

good point Tanguy, thanks for raising this. I guess we are better off waiting for your changes then.

I replicated the same test under the qa/rolling-upgrade project to verify the behavior. It fails in the line,

NodeJoinExecutor.ensureIndexCompatibility(NodeJoinExecutor.java:386)

when upgrading any of the nodes.

thanks for doing that!

I also got stuck in this area (IndexMetadataVerifier#checkSupportedVersion) when trying to revert one of the deleted FullClusterRestartIT tests. My quick fix of changing IndexVersions.MINIMUM_COMPATIBLE to IndexVersions.MINIMUM_READ_ONLY_COMPATIBLE in

elasticsearch/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java

Line 298 in 5ec0a84

IndexMetadata newMetadata = indexMetadataVerifier.verifyIndexMetadata(indexMetadata, IndexVersions.MINIMUM_COMPATIBLE);

led to other startup issues on full cluster restart, I haven't succeeded in exactly debugging those yet but will continue to investigate.

Sounds like we may need distrib help with some of these issues @cbuescher , or at least some of the changes they are working on.

I opened #118941 (with #118923 for 8.18) that should allow archive indices in 7.x to exist on a 9.x cluster. I will try to test the change for archive too but in the case you're interested here is the change for IndexMetadataVerifier.

…drempapis/elasticsearch into test/archive-index-version-compatibility

elasticsearchmachine · 2025-01-02T14:01:37Z

@drempapis according to this PR's labels, it should not have a changelog YAML, but I can't delete it because it is closed. Please either delete the changelog from the appropriate branch yourself, or adjust the labels.

javanna · 2025-01-06T14:43:49Z

docs/changelog/118599.yaml

+summary: Archive-Index upgrade compatibility
+area: Search
+type: enhancement
+issues: []


I believe that perhaps the changelog entry is not needed given that this became a test only change. It should probably be removed.

Yes, I accidentally added it and got notified. I created this tiny pr to fix it #119634

drempapis added 3 commits December 12, 2024 19:52

Add testing and refactoring

da8f1e2

Update code

713384f

update code

f69c5ff

drempapis self-assigned this Dec 12, 2024

elasticsearchmachine added the v9.0.0 label Dec 12, 2024

drempapis added >test Issues or PRs that are addressing/adding tests Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch :Search Foundations/Search Catch all for Search Foundations labels Dec 12, 2024

drempapis added 7 commits December 12, 2024 20:00

update code

f311c3d

Merge branch 'main' into test/archive-index-version-compatibility

78083bc

spotless execute

0b80ca6

Merge branch 'test/archive-index-version-compatibility' of github.com…

387bf82

…:drempapis/elasticsearch into test/archive-index-version-compatibility

Merge branch 'main' into test/archive-index-version-compatibility

5cfa128

Move project to other package and slight refactoring

6a3cceb

apply spotless

6c54c50

javanna reviewed Dec 16, 2024

View reviewed changes

drempapis added 3 commits December 16, 2024 13:17

update after review

9a63102

update after review

476328f

update after review

884a9e0

drempapis changed the title ~~Archive-Index version compatibility~~ Archive-Index upgrade compatibility Dec 16, 2024

drempapis mentioned this pull request Dec 16, 2024

Searchable snapshot upgrade compatibility #118770

Merged

drempapis added 2 commits December 16, 2024 16:21

Merge branch 'main' into test/archive-index-version-compatibility

79960b3

update after review

2fcb1ca

drempapis commented Dec 17, 2024

View reviewed changes

drempapis added 2 commits December 17, 2024 11:21

Merge branch 'main' into test/archive-index-version-compatibility

e8d6fee

update code

acec518

drempapis marked this pull request as ready for review December 17, 2024 09:56

Merge branch 'main' into test/archive-index-version-compatibility

b566c30

Merge branch 'main' into test/archive-index-version-compatibility

60ecd0b

javanna approved these changes Dec 17, 2024

View reviewed changes

javanna reviewed Dec 17, 2024

View reviewed changes

drempapis requested review from cbuescher and tlrx December 17, 2024 11:16

drempapis and others added 13 commits December 17, 2024 14:06

Merge branch 'main' into test/archive-index-version-compatibility

cbd9a4a

Update after review

df26c76

Merge branch 'main' into test/archive-index-version-compatibility

cd15012

Merge branch 'main' into test/archive-index-version-compatibility

10a227b

Merge branch 'main' into test/archive-index-version-compatibility

146334d

[CI] Auto commit changes from spotless

ece34cc

update after review

c395ddf

erge branch 'test/archive-index-version-compatibility' of github.com:…

1362eae

…drempapis/elasticsearch into test/archive-index-version-compatibility

Revert code

07f0553

Add changelog

817afbb

revert code

81333dd

Merge branch 'main' into test/archive-index-version-compatibility

8b6068c

Merge branch 'main' into test/archive-index-version-compatibility

57384ae

drempapis merged commit cd2ee4c into elastic:main Jan 2, 2025

javanna reviewed Jan 6, 2025

View reviewed changes

drempapis mentioned this pull request Jan 7, 2025

Archive index version compatibility remove changelog #119634

Merged


		import org.elasticsearch.test.cluster.util.Version;

		public class RestoreFromVersion5IT extends ArchiveIndexTestCase {

Conversation

drempapis commented Dec 12, 2024 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Dec 17, 2024

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javanna commented Dec 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jan 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drempapis Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

drempapis commented Dec 12, 2024 •

edited by atlassian bot

Loading

drempapis Jan 7, 2025 •

edited

Loading