System index descriptors support mixed versions #71144

jaymode · 2021-03-31T19:29:35Z

System index descriptors are used to describe a system index, which are
expected to change as new versions are developed. As part of this, the
descriptors had a minimum supported version field so that the contents
within that descriptor would not be applied if there were nodes older
than that version. However, this falls short of being able to
accurately describe what a system index should look like in a given
cluster where there are mixed node versions.

This change moves us towards being able to accurately describe and
know what the system index should look like. A system index is now
able to accept a list of the prior system index descriptor objects
so that clusters with mixed versions can select the appropriate
descriptor and ensure the index is created properly. As the node
versions change during a rolling upgrade, the cluster will then be
able to adapt the system index to the most recent version once all
master and data nodes have been upgraded.

System index descriptors are used to describe a system index, which are expected to change as new versions are developed. As part of this, the descriptors had a minimum supported version field so that the contents within that descriptor would not be applied if there were nodes older than that version. However, this falls short of being able to accurately describe what a system index should look like in a given cluster where there are mixed node versions. This change moves us towards being able to accurately describe and know what the system index should look like. A system index is now able to accept a list of the prior system index descriptor objects so that clusters with mixed versions can select the appropriate descriptor and ensure the index is created properly. As the node versions change during a rolling upgrade, the cluster will then be able to adapt the system index to the most recent version once all master and data nodes have been upgraded.

server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java

Co-authored-by: Tim Vernum <[email protected]>

ywangd

Looking good Jay! Thanks!

ywangd · 2021-04-01T03:44:45Z

...in/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

@@ -333,53 +333,63 @@ public void prepareIndexIfNeededThenExecute(final Consumer<Exception> consumer,
                    systemIndexDescriptor.getAliasName()
                );


Nit: This logging maybe not accurate sometimes if either descriptorForVersion has a different alias or descriptorForVersion is null.

...in/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

ywangd · 2021-04-01T05:25:28Z

@jaymode Maybe you already have an answer for it but did not show it here because the PR is still a draft. When I zoomed out of the current issue of mappings, I noticed that SecurityIndexManager still uses only the "main" systemIndexDescriptor to determine states in most places. A simple example is SecurityIndexManager#aliasName . It is not an issue for now because we have the same alias name across different versions. But it is in theory incorrect. The more complicated example is the SecurityIndexManager#clusterChanged method.

Two possible solutions that I can think of are:

Replace the usages with descriptorForVersion, though I am not sure how confident it is to say current descriptorForVersion is indeed the descriptor currently installed.
Disallow priorSystemIndexDescriptor to have different aliasName, indexFormat and primaryIndex. But it could be too constraining.

tvernum · 2021-04-01T05:32:25Z

Disallow priorSystemIndexDescriptor to have different aliasName, indexFormat and primaryIndex.

This would be my preference. It's simple, it has clearly understood behaviour & it meets all of the currently identified requirements.

Co-authored-by: Yang Wang <[email protected]>

jaymode · 2021-04-01T15:01:16Z

This would be my preference. It's simple, it has clearly understood behaviour & it meets all of the currently identified requirements.

+1. I knew about this gap and will address soon.

elasticmachine · 2021-04-02T20:27:11Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

williamrandolph

Looks good!

server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java

ywangd

LGTM

...in/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java

server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java

...in/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

Co-authored-by: Yang Wang <[email protected]>

System index descriptors are used to describe a system index, which are expected to change as new versions are developed. As part of this, the descriptors had a minimum supported version field so that the contents within that descriptor would not be applied if there were nodes older than that version. However, this falls short of being able to accurately describe what a system index should look like in a given cluster where there are mixed node versions. This change moves us towards being able to accurately describe and know what the system index should look like. A system index is now able to accept a list of the prior system index descriptor objects so that clusters with mixed versions can select the appropriate descriptor and ensure the index is created properly. As the node versions change during a rolling upgrade, the cluster will then be able to adapt the system index to the most recent version once all master and data nodes have been upgraded. Co-authored-by: Tim Vernum <[email protected]> Co-authored-by: Yang Wang <[email protected]> Backport of #71144

jaymode added 2 commits March 31, 2021 13:18

fix test descriptor

16b72d7

tvernum reviewed Mar 31, 2021

View reviewed changes

server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java Outdated Show resolved Hide resolved

tvernum reviewed Mar 31, 2021

View reviewed changes

server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java Outdated Show resolved Hide resolved

Apply suggestions from code review

38b76d9

Co-authored-by: Tim Vernum <[email protected]>

ywangd reviewed Apr 1, 2021

View reviewed changes

Apply suggestions from code review

d5bbbb8

Co-authored-by: Yang Wang <[email protected]>

jaymode added 6 commits April 1, 2021 09:06

Merge branch 'master' into prior_system_index_descriptors

0dc59d9

refinement and tests

6e983b6

Merge branch 'master' into prior_system_index_descriptors

39deef6

_doc is still around

0db92db

both ways

7e74d5c

fix test

2d445f5

jaymode marked this pull request as ready for review April 2, 2021 20:26

jaymode requested a review from williamrandolph April 2, 2021 20:26

jaymode added :Core/Infra/Core Core issues without another label >enhancement v7.13.0 v8.0.0 labels Apr 2, 2021

elasticmachine added the Team:Core/Infra Meta label for core/infra team label Apr 2, 2021

jaymode requested review from ywangd and tvernum April 2, 2021 20:27

williamrandolph approved these changes Apr 3, 2021

View reviewed changes

server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java Show resolved Hide resolved

ywangd approved these changes Apr 5, 2021

View reviewed changes

jaymode and others added 2 commits April 6, 2021 09:13

Apply suggestions from code review

13be0b4

Co-authored-by: Yang Wang <[email protected]>

dry up tests

b1f357c

jaymode added 5 commits April 6, 2021 09:42

better validation

a51fb40

cleanup version extraction

335d7ad

handle unmanaged indices that are internal

4c4132b

Merge branch 'master' into prior_system_index_descriptors

0c88cac

fix the oops

e2f841e

jaymode merged commit f9e9483 into elastic:master Apr 6, 2021

jaymode deleted the prior_system_index_descriptors branch April 6, 2021 19:47

jaymode added the backport pending label Apr 6, 2021

jaymode mentioned this pull request Apr 6, 2021

System index descriptors support mixed versions #71370

Merged

jaymode removed the backport pending label Apr 7, 2021

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

System index descriptors support mixed versions #71144

System index descriptors support mixed versions #71144

jaymode commented Mar 31, 2021

ywangd left a comment

ywangd Apr 1, 2021

ywangd commented Apr 1, 2021

tvernum commented Apr 1, 2021

jaymode commented Apr 1, 2021

elasticmachine commented Apr 2, 2021

williamrandolph left a comment

ywangd left a comment

		@@ -333,53 +333,63 @@ public void prepareIndexIfNeededThenExecute(final Consumer<Exception> consumer,
		systemIndexDescriptor.getAliasName()
		);

System index descriptors support mixed versions #71144

System index descriptors support mixed versions #71144

Conversation

jaymode commented Mar 31, 2021

ywangd left a comment

Choose a reason for hiding this comment

ywangd Apr 1, 2021

Choose a reason for hiding this comment

ywangd commented Apr 1, 2021

tvernum commented Apr 1, 2021

jaymode commented Apr 1, 2021

elasticmachine commented Apr 2, 2021

williamrandolph left a comment

Choose a reason for hiding this comment

ywangd left a comment

Choose a reason for hiding this comment