Ensure consistency of system flag on IndexMetadata after diff is applied #16644
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Craig Perkins <[email protected]>
cwperks
requested review from
anasalkouz,
andrross,
ashking94,
Bukhtawar,
CEHENKLE,
dblock,
dbwiddis,
gbbafna,
jainankitk,
kotwanikunal,
linuxpi,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
Bukhtawar
approved these changes
Nov 14, 2024
shwetathareja
approved these changes
Nov 14, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16644 +/- ##
============================================
- Coverage 72.51% 72.04% -0.48%
+ Complexity 65562 65169 -393
============================================
Files 5318 5318
Lines 303945 303945
Branches 43976 43976
============================================
- Hits 220413 218966 -1447
- Misses 65798 67002 +1204
- Partials 17734 17977 +243 ☔ View full report in Codecov by Sentry. |
|
@cwperks this change needs a changelog entry |
Signed-off-by: Craig Perkins <[email protected]>
Added |
|
Resolved conflicts in CHANGELOG |
|
Resolved conflicts on CHANGELOG again. Can a maintainer help to merge this PR? |
| @@ -88,6 +92,36 @@ protected NamedXContentRegistry xContentRegistry() { | |||
| return new NamedXContentRegistry(IndicesModule.getNamedXContents()); | |||
| } | |||
|
|
|||
| // Create the index metadata for a given index, with the specified version. | |||
| private static IndexMetadata createIndexMetadata(final Index index, final long version) { | |||
There was a problem hiding this comment.
nitpick on code refactoring:
private static IndexMetadata createIndexMetadata(final Index index, final long version) {
return createIndexMetadata(index, version, false (or the default value));
}
unless setting to default vs not setting the .system() achieves a different functionality?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR ensures the consistency of the
isSystemflag between leader and follower after the IndexMetadataDiff is applied when computing new cluster state.This inconsistency can appear during rolling upgrades if nodes of a new version know of an index as a system index, but nodes of a previous version treat the same index as a regular index. For example, if a plugin retroactively declares an index to be a system index through SystemIndexPlugin.getSystemIndexDescriptors (Example PR) there can be a discrepancy in the IndexMetadata (for the same version) between the cluster manager node and other nodes in the cluster.
The reason this happens is because when the IndexMetadataDiff is applied, its taking the value from the previous metadata on the node instead of the new metadata that it has received from the incoming publishRequest. As a result, the diff here is being computed incorrectly. It should take the value from the diff instead of from the previous index metadata.
Related Issues
Resolves #16643
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.