Browse Source

Revert "Fail index creation using custom data path (#76792)" (#78031)

This reverts commit 79d91ed9d3b72c19ccd8ab3bd32e0833a8c47e8f.
Ryan Ernst 4 years ago
parent
commit
a06aff9b01

+ 0 - 14
docs/reference/migration/migrate_8_0/indices.asciidoc

@@ -116,18 +116,4 @@ index setting].
 Accept the new behaviour, or specify `?wait_for_active_shards=0` to preserve
 the old behaviour if needed.
 ====
-
-.The setting `index.data_path` is not longer allowed on new indices.
-[%collapsible]
-====
-*Details* +
-Creating an index relative to the node level `path.shared_data` setting was
-previously used with shadow replicas prior to their removal in 6.0. The
-per-index data path in `index.data_path` was deprecated in 7.14.0. In 8.0,
-creating new indices with `index.data_path` is no longer supported.
-
-*Impact* +
-Discontinue use of the `index.data_path` setting. Creating new indices with
-this setting will return an error.
-====
 //end::notable-breaking-changes[]

+ 2 - 17
server/src/internalClusterTest/java/org/elasticsearch/index/shard/IndexShardIT.java

@@ -66,7 +66,6 @@ import org.elasticsearch.test.DummyShardLock;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.test.IndexSettingsModule;
 import org.elasticsearch.test.InternalSettingsPlugin;
-import org.elasticsearch.test.VersionUtils;
 import org.junit.Assert;
 
 import java.io.IOException;
@@ -117,11 +116,6 @@ public class IndexShardIT extends ESSingleNodeTestCase {
         return pluginList(InternalSettingsPlugin.class);
     }
 
-    @Override
-    protected boolean forbidPrivateIndexSettings() {
-        return false;
-    }
-
     public void testLockTryingToDelete() throws Exception {
         createIndex("test");
         ensureGreen();
@@ -220,11 +214,8 @@ public class IndexShardIT extends ESSingleNodeTestCase {
         Environment env = getInstanceFromNode(Environment.class);
         Path idxPath = env.sharedDataFile().resolve(randomAlphaOfLength(10));
         logger.info("--> idxPath: [{}]", idxPath);
-        Version createdVersion =
-            VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion(Version.V_8_0_0));
         Settings idxSettings = Settings.builder()
             .put(IndexMetadata.SETTING_DATA_PATH, idxPath)
-            .put(IndexMetadata.SETTING_VERSION_CREATED, createdVersion)
             .build();
         createIndex("test", idxSettings);
         ensureGreen("test");
@@ -262,14 +253,8 @@ public class IndexShardIT extends ESSingleNodeTestCase {
         final Path sharedDataPath = getInstanceFromNode(Environment.class).sharedDataFile().resolve(randomAsciiLettersOfLength(10));
         final Path indexDataPath = sharedDataPath.resolve("start-" + randomAsciiLettersOfLength(10));
 
-        logger.info("--> creating legacy index [{}] with data_path [{}]", index, indexDataPath);
-        Version createdVersion =
-            VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion(Version.V_8_0_0));
-        Settings settings = Settings.builder()
-            .put(IndexMetadata.SETTING_DATA_PATH, indexDataPath.toAbsolutePath().toString())
-            .put(IndexMetadata.SETTING_VERSION_CREATED, createdVersion)
-            .build();
-        createIndex(index, settings);
+        logger.info("--> creating index [{}] with data_path [{}]", index, indexDataPath);
+        createIndex(index, Settings.builder().put(IndexMetadata.SETTING_DATA_PATH, indexDataPath.toAbsolutePath().toString()).build());
         client().prepareIndex(index).setId("1").setSource("foo", "bar").setRefreshPolicy(IMMEDIATE).get();
         ensureGreen(index);
 

+ 20 - 8
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java

@@ -48,6 +48,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexModule;
@@ -68,6 +69,7 @@ import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.nio.file.Path;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -794,7 +796,6 @@ public class MetadataCreateIndexService {
         validateSoftDeleteSettings(indexSettings);
         validateTranslogRetentionSettings(indexSettings);
         validateStoreTypeSetting(indexSettings);
-        validateNoCustomPath(indexSettings);
         return indexSettings;
     }
 
@@ -1046,7 +1047,7 @@ public class MetadataCreateIndexService {
     }
 
     List<String> getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) {
-        List<String> validationErrors = new ArrayList<>();
+        List<String> validationErrors = validateIndexCustomPath(settings, env.sharedDataFile());
         if (forbidPrivateIndexSettings) {
             validationErrors.addAll(validatePrivateSettingsNotExplicitlySet(settings, indexScopedSettings));
         }
@@ -1067,16 +1068,27 @@ public class MetadataCreateIndexService {
     }
 
     /**
-     * Validates an index data path is not specified.
+     * Validates that the configured index data path (if any) is a sub-path of the configured shared data path (if any)
      *
      * @param settings the index configured settings
+     * @param sharedDataPath the configured `path.shared_data` (if any)
+     * @return a list containing validaton errors or an empty list if there aren't any errors
      */
-    static void validateNoCustomPath(Settings settings) {
-        if (IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(Version.V_8_0_0) &&
-            IndexMetadata.INDEX_DATA_PATH_SETTING.exists(settings)) {
-            throw new IllegalArgumentException("per-index custom data path using setting ["
-                + IndexMetadata.INDEX_DATA_PATH_SETTING.getKey() + "] is no longer supported on new indices");
+    private static List<String> validateIndexCustomPath(Settings settings, @Nullable Path sharedDataPath) {
+        String customPath = IndexMetadata.INDEX_DATA_PATH_SETTING.get(settings);
+        List<String> validationErrors = new ArrayList<>();
+        if (Strings.isEmpty(customPath) == false) {
+            if (sharedDataPath == null) {
+                validationErrors.add("path.shared_data must be set in order to use custom data paths");
+            } else {
+                Path resolvedPath = PathUtils.get(new Path[]{sharedDataPath}, customPath);
+                if (resolvedPath == null) {
+                    validationErrors.add("custom path [" + customPath +
+                        "] is not a sub-path of path.shared_data [" + sharedDataPath + "]");
+                }
+            }
         }
+        return validationErrors;
     }
 
     /**

+ 2 - 14
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java

@@ -34,10 +34,10 @@ import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentFactory;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.index.IndexNotFoundException;
@@ -91,9 +91,9 @@ import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.clus
 import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.getIndexNumberOfRoutingShards;
 import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.parseV1Mappings;
 import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.resolveAndValidateAliases;
+
 import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING;
 import static org.elasticsearch.indices.ShardLimitValidatorTests.createTestShardLimitService;
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
@@ -281,18 +281,6 @@ public class MetadataCreateIndexServiceTests extends ESTestCase {
             Settings.builder().put("index.number_of_shards", targetShards).build());
     }
 
-    public void testValidateNoCustomPath() {
-        Settings indexSettings = Settings.builder()
-            .put(SETTING_VERSION_CREATED, Version.V_8_0_0)
-            .put(IndexMetadata.INDEX_DATA_PATH_SETTING.getKey(), "some/path")
-            .build();
-        var e = expectThrows(IllegalArgumentException.class,
-            () -> MetadataCreateIndexService.validateNoCustomPath(indexSettings));
-        assertThat(e.getMessage(), containsString("per-index custom data path"));
-
-        MetadataCreateIndexService.validateNoCustomPath(Settings.EMPTY);
-    }
-
     public void testPrepareResizeIndexSettings() {
         final List<Version> versions = Arrays.asList(VersionUtils.randomVersion(random()), VersionUtils.randomVersion(random()));
         versions.sort(Comparator.comparingLong(l -> l.id));

+ 6 - 0
test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java

@@ -686,6 +686,12 @@ public abstract class ESIntegTestCase extends ESTestCase {
         if (numberOfReplicas >= 0) {
             builder.put(SETTING_NUMBER_OF_REPLICAS, numberOfReplicas).build();
         }
+        // 30% of the time
+        if (randomInt(9) < 3) {
+            final String dataPath = randomAlphaOfLength(10);
+            logger.info("using custom data_path for index: [{}]", dataPath);
+            builder.put(IndexMetadata.SETTING_DATA_PATH, dataPath);
+        }
         // always default delayed allocation to 0 to make sure we have tests are not delayed
         builder.put(UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), 0);
         if (randomBoolean()) {

+ 2 - 1
x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java

@@ -234,11 +234,12 @@ public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnaps
 
     protected void assertShardFolders(String indexName, boolean snapshotDirectory) throws IOException {
         final Index restoredIndex = resolveIndex(indexName);
+        final String customDataPath = resolveCustomDataPath(indexName);
         final ShardId shardId = new ShardId(restoredIndex, 0);
         boolean shardFolderFound = false;
         for (String node : internalCluster().getNodeNames()) {
             final NodeEnvironment service = internalCluster().getInstance(NodeEnvironment.class, node);
-            final ShardPath shardPath = ShardPath.loadShardPath(logger, service, shardId, null);
+            final ShardPath shardPath = ShardPath.loadShardPath(logger, service, shardId, customDataPath);
             if (shardPath != null && Files.exists(shardPath.getDataPath())) {
                 shardFolderFound = true;
                 assertEquals(snapshotDirectory, Files.notExists(shardPath.resolveIndex()));