-
Notifications
You must be signed in to change notification settings - Fork 25.6k
IndexMetaData: Introduce internal format index setting #25292
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
IndexMetaData: Introduce internal format index setting #25292
Conversation
This setting is supposed to ease index upgrades as it allows you to check for a new setting called `index.internal.version` which in turn can be configured in the index settings and has to be set to '6' in order to be valid.
s1monw
left a comment
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.
left some comments
| private static final String INDEX_INTERNAL_FORMAT = "index.internal.format"; | ||
| public static final Setting<Integer> INDEX_INTERNAL_FORMAT_SETTING = | ||
| Setting.intSetting(INDEX_INTERNAL_FORMAT, 0, Setting.Property.IndexScope); | ||
| static final int INTERNAL_INDEX_FORMAT_CURRENT = 6; |
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.
maybe we make this a bit more generic and call it index.format and INTERNAL_INDEX_FORMAT_CURRENT should not be here. it's up to every user of this setting to define the current?
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.
fixed
| private final Index index; | ||
| private final long version; | ||
| private final long[] primaryTerms; | ||
| private final int internalIndexFormat; |
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.
do we really have to make this a first class citizen or can we just leave it as a setting?
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.
makes more sense as a setting. fixed
| * an internal index format description, allowing us to find out if this index is upgraded or needs upgrading | ||
| */ | ||
| private static final String INDEX_INTERNAL_FORMAT = "index.internal.format"; | ||
| public static final Setting<Integer> INDEX_INTERNAL_FORMAT_SETTING = |
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.
also make this setting Setting.Property.Final
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.
oooh, wasnt aware of that. great!
- Make the setting final - Do not expose an own getter, just extract the setting via IndexMetaData.getSettings() - also no need to put this into hashCode()/equals() - Make more generic and rename to `index.format` - Allow the user to choose the correct value and just return that value
|
all comments made a ton of sense. incorporated. |
s1monw
left a comment
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.
left some comments
| DiscoveryNodeFilters requireFilters, DiscoveryNodeFilters initialRecoveryFilters, DiscoveryNodeFilters includeFilters, DiscoveryNodeFilters excludeFilters, | ||
| Version indexCreatedVersion, Version indexUpgradedVersion, | ||
| int routingNumShards, int routingPartitionSize, ActiveShardCount waitForActiveShards) { | ||
| int routingNumShards, int routingPartitionSize, ActiveShardCount waitForActiveShards, int internalIndexFormat) { |
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 is unused?
|
|
||
| final String uuid = settings.get(SETTING_INDEX_UUID, INDEX_UUID_NA_VALUE); | ||
|
|
||
| int internalIndexFormat = INDEX_FORMAT_SETTING.get(settings); |
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 is unneeded see above
| return new IndexMetaData(new Index(index, uuid), version, primaryTerms, state, numberOfShards, numberOfReplicas, tmpSettings, mappings.build(), | ||
| tmpAliases.build(), customs.build(), filledInSyncAllocationIds.build(), requireFilters, initialRecoveryFilters, includeFilters, excludeFilters, | ||
| indexCreatedVersion, indexUpgradedVersion, getRoutingNumShards(), routingPartitionSize, waitForActiveShards); | ||
| indexCreatedVersion, indexUpgradedVersion, getRoutingNumShards(), routingPartitionSize, waitForActiveShards, internalIndexFormat); |
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 should go away
s1monw
left a comment
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
This setting is supposed to ease index upgrades as it allows you to check for a new setting called `index.internal.version` which can be used to check before upgrading indices.
* master: (56 commits) Initialize max unsafe auto ID timestamp on shrink Enable a long translog retention policy by default (elastic#25294) Remove `index.mapping.single_type=false` from core/tests (elastic#25331) test: single type defaults to true since alpha1 and not alpha3 Get short path name for native controllers Live primary-replica resync (no rollback) (elastic#24841) Upgrade to lucene-7.0.0-snapshot-ad2cb77. (elastic#25349) percolator: Deprecate `document_type` parameter. [DOCS] Fixed typo. [rest-api-spec/indices.refresh] Remove old params Remove redundant and broken MD5 checksum from repository-s3 (elastic#25270) Initialize sequence numbers on a shrunken index Port most snapshot/restore static bwc tests to qa:full-cluster-restart (elastic#25296) Javadoc: ThreadPool doesn't reject while shutdown (elastic#23678) test: verify `size_to_upgrade_in_bytes` in assertBusy(...) Docs: Removed duplicated line in mapping docs Add backward compatibility indices for 5.4.2 Update MockTransportService to the age of Transport.Connection (elastic#25320) Add version v5.4.2 after release IndexMetaData: Add internal format index setting (elastic#25292) ...
This setting is supposed to ease index upgrades as it allows you
to check for a new setting called
index.internal.versionwhichin turn can be configured in the index settings and has to be set
to '6' in order to be valid.