Skip to content

Commit

Permalink
Reduce ways of checking index version setting. (#65396)
Browse files Browse the repository at this point in the history
This PR removes the public method `Version#indexCreated(Settings)`. Now, the
index version is always checked through `IndexSettings#indexCreatedVersion`,
except in rare lower-level cases where the setting is accessed directly.

This makes it easier to understand what logic depends on the index version.
It's a step towards the goal of streamlining how/ where index versions are
accessed.
  • Loading branch information
jtibshirani committed Nov 30, 2020
1 parent da4488e commit b6d761e
Show file tree
Hide file tree
Showing 22 changed files with 95 additions and 98 deletions.
21 changes: 0 additions & 21 deletions server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@

package org.elasticsearch;

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.collect.ImmutableOpenIntMap;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.monitor.jvm.JvmInfo;
Expand Down Expand Up @@ -171,25 +169,6 @@ private static Version fromIdSlow(int id) {
return new Version(id, luceneVersion);
}

/**
* Return the {@link Version} of Elasticsearch that has been used to create an index given its settings.
*
* @throws IllegalStateException if the given index settings doesn't contain a value for the key
* {@value IndexMetadata#SETTING_VERSION_CREATED}
*/
public static Version indexCreated(Settings indexSettings) {
final Version indexVersion = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings);
if (indexVersion == V_EMPTY) {
final String message = String.format(
Locale.ROOT,
"[%s] is not present in the index settings for index with UUID [%s]",
IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(),
indexSettings.get(IndexMetadata.SETTING_INDEX_UUID));
throw new IllegalStateException(message);
}
return indexVersion;
}

public static void writeVersion(Version version, StreamOutput out) throws IOException {
out.writeVInt(version.id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ public IndexMetadata build() {
} else {
initialRecoveryFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, initialRecoveryMap);
}
Version indexCreatedVersion = Version.indexCreated(settings);
Version indexCreatedVersion = indexCreatedVersion(settings);
Version indexUpgradedVersion = settings.getAsVersion(IndexMetadata.SETTING_VERSION_UPGRADED, indexCreatedVersion);

if (primaryTerms == null) {
Expand Down Expand Up @@ -1531,13 +1531,34 @@ public static IndexMetadata fromXContent(XContentParser parser) throws IOExcepti
if (Assertions.ENABLED) {
assert settingsVersion : "settings version should be present for indices created on or after 6.5.0";
}
if (Assertions.ENABLED && Version.indexCreated(builder.settings).onOrAfter(Version.V_7_2_0)) {

Version indexVersion = indexCreatedVersion(builder.settings);
if (Assertions.ENABLED && indexVersion.onOrAfter(Version.V_7_2_0)) {
assert aliasesVersion : "aliases version should be present for indices created on or after 7.2.0";
}
return builder.build();
}
}

/**
* Return the {@link Version} of Elasticsearch that has been used to create an index given its settings.
*
* @throws IllegalArgumentException if the given index settings doesn't contain a value for the key
* {@value IndexMetadata#SETTING_VERSION_CREATED}
*/
private static Version indexCreatedVersion(Settings indexSettings) {
final Version indexVersion = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings);
if (indexVersion == Version.V_EMPTY) {
final String message = String.format(
Locale.ROOT,
"[%s] is not present in the index settings for index with UUID [%s]",
IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(),
indexSettings.get(IndexMetadata.SETTING_INDEX_UUID));
throw new IllegalArgumentException(message);
}
return indexVersion;
}

/**
* Adds human readable version and creation date settings.
* This method is used to display the settings in a human readable format in REST API
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -676,10 +676,10 @@ static Settings aggregateIndexSettings(ClusterState currentState, CreateIndexClu
// now, put the request settings, so they override templates
indexSettingsBuilder.put(requestSettings.build());

if (indexSettingsBuilder.get(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey()) == null) {
if (indexSettingsBuilder.get(IndexMetadata.SETTING_VERSION_CREATED) == null) {
final DiscoveryNodes nodes = currentState.nodes();
final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion());
indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion);
indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, createdVersion);
}
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(indexSettingsBuilder) == false) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, INDEX_NUMBER_OF_SHARDS_SETTING.get(settings));
Expand Down Expand Up @@ -1129,7 +1129,7 @@ static void prepareResizeIndexSettings(
}

indexSettingsBuilder
.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), sourceMetadata.getCreationVersion())
.put(IndexMetadata.SETTING_VERSION_CREATED, sourceMetadata.getCreationVersion())
.put(IndexMetadata.SETTING_VERSION_UPGRADED, sourceMetadata.getUpgradedVersion())
.put(builder.build())
.put(IndexMetadata.SETTING_ROUTING_PARTITION_SIZE, sourceMetadata.getRoutingPartitionSize())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ public String customDataPath() {

/**
* Returns the version the index was created on.
* @see Version#indexCreated(Settings)
* @see IndexMetadata#SETTING_VERSION_CREATED
*/
public Version getIndexVersionCreated() {
return version;
Expand Down Expand Up @@ -676,9 +676,9 @@ public Settings getNodeSettings() {
*/
public synchronized boolean updateIndexMetadata(IndexMetadata indexMetadata) {
final Settings newSettings = indexMetadata.getSettings();
if (version.equals(Version.indexCreated(newSettings)) == false) {
throw new IllegalArgumentException("version mismatch on settings update expected: " + version + " but was: " +
Version.indexCreated(newSettings));
Version newIndexVersion = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(newSettings);
if (version.equals(newIndexVersion) == false) {
throw new IllegalArgumentException("version mismatch on settings update expected: " + version + " but was: " + newIndexVersion);
}
final String newUUID = newSettings.get(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE);
if (newUUID.equals(getUUID()) == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public abstract class AbstractIndexAnalyzerProvider<T extends Analyzer> extends
public AbstractIndexAnalyzerProvider(IndexSettings indexSettings, String name, Settings settings) {
super(indexSettings);
this.name = name;
this.version = Analysis.parseAnalysisVersion(this.indexSettings.getSettings(), settings, logger);
this.version = Analysis.parseAnalysisVersion(this.indexSettings, settings, logger);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ public abstract class AbstractTokenFilterFactory extends AbstractIndexComponent
public AbstractTokenFilterFactory(IndexSettings indexSettings, String name, Settings settings) {
super(indexSettings);
this.name = name;
this.version = Analysis.parseAnalysisVersion(this.indexSettings.getSettings(), settings, logger);
this.version = Analysis.parseAnalysisVersion(indexSettings, settings, logger);
}

@Override
public String name() {
return this.name;
}

public final Version version() {
return version;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public abstract class AbstractTokenizerFactory extends AbstractIndexComponent im

public AbstractTokenizerFactory(IndexSettings indexSettings, Settings settings, String name) {
super(indexSettings);
this.version = Analysis.parseAnalysisVersion(this.indexSettings.getSettings(), settings, logger);
this.version = Analysis.parseAnalysisVersion(indexSettings, settings, logger);
this.name = name;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.IndexSettings;

import java.io.BufferedReader;
import java.io.IOException;
Expand All @@ -79,19 +80,19 @@

public class Analysis {

public static Version parseAnalysisVersion(Settings indexSettings, Settings settings, Logger logger) {
public static Version parseAnalysisVersion(IndexSettings indexSettings, Settings settings, Logger logger) {
// check for explicit version on the specific analyzer component
String sVersion = settings.get("version");
if (sVersion != null) {
return Lucene.parseVersion(sVersion, Version.LATEST, logger);
}
// check for explicit version on the index itself as default for all analysis components
sVersion = indexSettings.get("index.analysis.version");
sVersion = indexSettings.getSettings().get("index.analysis.version");
if (sVersion != null) {
return Lucene.parseVersion(sVersion, Version.LATEST, logger);
}
// resolve the analysis version based on the version the index was created with
return org.elasticsearch.Version.indexCreated(indexSettings).luceneVersion;
return indexSettings.getIndexVersionCreated().luceneVersion;
}

public static CharArraySet parseStemExclusion(Settings settings, CharArraySet defaultStemExclusion) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public AnalyzerProvider<?> get(IndexSettings indexSettings,
Environment environment,
String name,
Settings settings) throws IOException {
Version versionCreated = Version.indexCreated(settings);
Version versionCreated = indexSettings.getIndexVersionCreated();
if (Version.CURRENT.equals(versionCreated) == false) {
return super.get(indexSettings, environment, name, settings);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protected PreConfiguredAnalysisComponent(String name, PreBuiltCacheFactory.PreBu

@Override
public T get(IndexSettings indexSettings, Environment environment, String name, Settings settings) throws IOException {
Version versionCreated = Version.indexCreated(settings);
Version versionCreated = indexSettings.getIndexVersionCreated();
synchronized (this) {
T factory = cache.get(versionCreated);
if (factory == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,15 @@ private static void innerParseObject(ParseContext context, ObjectMapper mapper,
private static void nested(ParseContext context, ObjectMapper.Nested nested) {
ParseContext.Document nestedDoc = context.doc();
ParseContext.Document parentDoc = nestedDoc.getParent();
Settings settings = context.indexSettings().getSettings();
Version indexVersion = context.indexSettings().getIndexVersionCreated();
if (nested.isIncludeInParent()) {
addFields(Version.indexCreated(settings), nestedDoc, parentDoc);
addFields(indexVersion, nestedDoc, parentDoc);
}
if (nested.isIncludeInRoot()) {
ParseContext.Document rootDoc = context.rootDoc();
// don't add it twice, if its included in parent, and we are handling the master doc...
if (!nested.isIncludeInParent() || parentDoc != rootDoc) {
addFields(Version.indexCreated(settings), nestedDoc, rootDoc);
addFields(indexVersion, nestedDoc, rootDoc);
}
}
}
Expand Down Expand Up @@ -463,7 +463,7 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map
throw new IllegalStateException("The root document of a nested document should have an _id field");
}

Version version = Version.indexCreated(context.indexSettings().getSettings());
Version version = context.indexSettings().getIndexVersionCreated();
nestedDoc.add(NestedPathFieldMapper.field(version, mapper.nestedTypePath()));
return context;
}
Expand Down Expand Up @@ -502,7 +502,7 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
} else if (dynamic == ObjectMapper.Dynamic.TRUE) {
Mapper.Builder builder = context.root().findTemplateBuilder(context, currentFieldName, XContentFieldType.OBJECT);
if (builder == null) {
Version version = Version.indexCreated(context.indexSettings().getSettings());
Version version = context.indexSettings().getIndexVersionCreated();
builder = new ObjectMapper.Builder(currentFieldName, version).enabled(true);
}
objectMapper = builder.build(context.path());
Expand Down Expand Up @@ -684,7 +684,7 @@ private static Mapper.Builder createBuilderFromDynamicValue(final ParseContext c
if (builder == null) {
boolean ignoreMalformed = IGNORE_MALFORMED_SETTING.get(context.indexSettings().getSettings());
builder = new DateFieldMapper.Builder(currentFieldName, DateFieldMapper.Resolution.MILLISECONDS,
dateTimeFormatter, ignoreMalformed, Version.indexCreated(context.indexSettings().getSettings()));
dateTimeFormatter, ignoreMalformed, context.indexSettings().getIndexVersionCreated());
}
return builder;

Expand Down Expand Up @@ -835,7 +835,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(ParseContext
case TRUE:
Mapper.Builder builder = context.root().findTemplateBuilder(context, paths[i], XContentFieldType.OBJECT);
if (builder == null) {
Version version = Version.indexCreated(context.indexSettings().getSettings());
Version version = context.indexSettings().getIndexVersionCreated();
builder = new ObjectMapper.Builder(paths[i], version).enabled(true);
}
mapper = (ObjectMapper) builder.build(context.path());
Expand Down
17 changes: 0 additions & 17 deletions server/src/test/java/org/elasticsearch/VersionTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@

package org.elasticsearch;

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.VersionUtils;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -149,21 +147,6 @@ public void testWrongVersionFromString() {
assertThat(e.getMessage(), containsString("needs to contain major, minor, and revision"));
}

public void testVersionNoPresentInSettings() {
Exception e = expectThrows(IllegalStateException.class, () -> Version.indexCreated(Settings.builder().build()));
assertThat(e.getMessage(), containsString("[index.version.created] is not present"));
}

public void testIndexCreatedVersion() {
// an actual index has a IndexMetadata.SETTING_INDEX_UUID
final Version version = VersionUtils.randomVersion(random());
assertEquals(version, Version.indexCreated(
Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, "foo")
.put(IndexMetadata.SETTING_VERSION_CREATED, version)
.build()));
}

public void testMinCompatVersion() {
Version major = Version.fromString("2.0.0");
assertThat(Version.fromString("2.0.0").minimumCompatibilityVersion(), equalTo(major));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,15 @@ private void runTestNumberOfShardsIsPositive(final int numberOfShards) {
equalTo("Failed to parse value [" + numberOfShards + "] for setting [index.number_of_shards] must be >= 1"));
}

public void testMissingCreatedVersion() {
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
IndexMetadata.builder("test").settings(settings).build());
assertThat(e.getMessage(), containsString("[index.version.created] is not present"));
}

public void testMissingNumberOfReplicas() {
final Settings settings =
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 8)).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_VERSION_CREATED;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.aggregateIndexSettings;
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.buildIndexMetadata;
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.clusterStateCreateIndex;
Expand Down Expand Up @@ -121,7 +121,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase {
public void setupCreateIndexRequestAndAliasValidator() {
aliasValidator = new AliasValidator();
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
Settings indexSettings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build();
queryShardContext = new QueryShardContext(0,
new IndexSettings(IndexMetadata.builder("test").settings(indexSettings).build(), indexSettings),
Expand Down Expand Up @@ -900,7 +900,7 @@ public void testRejectTranslogRetentionSettings() {
settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), between(1, 128) + "mb");
}
if (randomBoolean()) {
settings.put(SETTING_INDEX_VERSION_CREATED.getKey(),
settings.put(SETTING_VERSION_CREATED,
VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, Version.CURRENT));
}
request.settings(settings.build());
Expand All @@ -920,7 +920,7 @@ public void testDeprecateTranslogRetentionSettings() {
} else {
settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), between(1, 128) + "mb");
}
settings.put(SETTING_INDEX_VERSION_CREATED.getKey(), VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0));
settings.put(SETTING_VERSION_CREATED, VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0));
request.settings(settings.build());
aggregateIndexSettings(ClusterState.EMPTY_STATE, request, Settings.EMPTY,
null, Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(),
Expand Down

0 comments on commit b6d761e

Please sign in to comment.