-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Introduce index settings version #34429
Conversation
This commit introduces settings version to index metadata. This value is monotonically increasing and is updated on settings updates. This will be useful in cross-cluster replication so that we can request settings updates from the leader only when there is a settings update.
Pinging @elastic/es-core-infra |
Pinging @elastic/es-distributed |
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.
this LGTM - I wonder if we should make this entire version game here in IMD less error prone. I don't have a good answer but now we have 3 version that are all can be changed and incremented separately I do wonder if we should streamline it at least with a Version class that only can move forward?
public Builder mappingVersion(final long mappingVersion) { | ||
this.mappingVersion = mappingVersion; | ||
return this; | ||
} | ||
|
||
public Builder settingsVersion(final long settingsVersion) { | ||
this.settingsVersion = settingsVersion; |
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.
would it make sense to check that we always increase?
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.
That would not be correct since this method is called when building index metadata from parsing or de-serializing.
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.
This looks good. I also do think that we need to update the index settings version in IndexMetaData.Builder#numberOfReplicas(...)
method. This is used by AllocationService#adaptAutoExpandReplicas(...)
and I think the reason the UpdateNumberOfReplicasIT
fails.
if (same(currentState.metaData().index(index).getSettings(), metaDataBuilder.get(index).getSettings()) == false) { | ||
final IndexMetaData.Builder builder = IndexMetaData.builder(metaDataBuilder.get(index)); | ||
builder.settingsVersion(1 + builder.settingsVersion()); | ||
metaDataBuilder.put(builder); |
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.
can we instead increment the version at lines 169 and 185? like:
metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(finalSettings).settingsVersion(1 + builder.settingsVersion()));
At these places we do know that the index versions have been updated. And then we don't have to check whether settings have been changed in another for loop.
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.
That would not be correct since a settings update does not necessarily mutate any settings. The test UpdateSettingsIT#testSettingsVersionUnchanged
test for this case. That is, a settings update that does not change any settings should not modify the settings version.
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.
thanks for adding that test!
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
* elastic/master: Mute PartitionedRoutingIT#testShrinking on Windows Mute testToQuery test [TEST] Make sure there are shards started so that `ESIntegTestCase#assertSameDocIdsOnShards()` does not fail with shard not found. Change shard changes api's threadpool from get to search (elastic#34421) Update TESTING.asciidoc title (elastic#34401) Tests: Fix DateFormatter equals tests with locale (elastic#34435) Docs: Remove unnecessary qualifier from wildcard import note (elastic#34419) CCR/TEST: AwaitsFix testFailOverOnFollower [Painless] Add a Map for java names to classes for use in the custom classloader (elastic#34424) TEST: Fix indentation in FullClusterRestartIT (elastic#34420) [WIP] Ingest Attachement: Upgrade tika to v1.19.1 (elastic#33896) NETWORKING: Upgrade Netty to 4.1.30 (elastic#34417) Allow an AuthenticationResult to return metadata (elastic#34382) [ML] Add an ingest pipeline definition to structure finder (elastic#34350) Handle pre-6.x time fields (elastic#34373) ListenableFuture should preserve ThreadContext (elastic#34394)
This test failure here is fixed by #34481. |
* master: Do not update number of replicas on no indices (elastic#34481) Core: Remove two methods from AbstractComponent (elastic#34336) Use RoleRetrievalResult for better caching (elastic#34197) Revert "Search: Fix spelling mistake in Javadoc (elastic#34480)" Search: Fix spelling mistake in Javadoc (elastic#34480) Docs: improve formatting of Query String Query doc page (elastic#34432) Tests: Handle epoch date formatters edge cases (elastic#34437) Test: Fix running with external cluster Fix handling of empty keyword in terms aggregation (elastic#34457) [DOCS] Fix tag in SecurityDocumentationIT [Monitoring] Add additional necessary mappings for apm-server (elastic#34392) SCRIPTING: Move Aggregation Script Context to its own class (elastic#33820) MINOR: Remove Deadcode in ExpressionTermSetQuery (elastic#34442) HLRC: Get SSL Certificates API (elastic#34135)
This commit introduces settings version to index metadata. This value is monotonically increasing and is updated on settings updates. This will be useful in cross-cluster replication so that we can request settings updates from the leader only when there is a settings update.
This commit introduces settings version to index metadata. This value is monotonically increasing and is updated on settings updates. This will be useful in cross-cluster replication so that we can request settings updates from the leader only when there is a settings update.
This commit introduces settings version to index metadata. This value is monotonically increasing and is updated on settings updates. This will be useful in cross-cluster replication so that we can request settings updates from the leader only when there is a settings update.