From b58970a3fed699c391c0c5f008314e06dea897ae Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 12 Aug 2015 15:58:36 -0600 Subject: [PATCH] [TEST] Remove custom data path tests, replace with better tests Replace with a better, "unit-y" tests that couple the shared data path with the correctly set `path.shared_data` configuration setting --- .../index/shard/IndexShardTests.java | 90 ++++++++++ .../indices/IndicesCustomDataPathIT.java | 164 ------------------ .../elasticsearch/test/ESIntegTestCase.java | 33 ---- .../org/elasticsearch/test/ESTestCase.java | 35 ++++ 4 files changed, 125 insertions(+), 197 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/indices/IndicesCustomDataPathIT.java diff --git a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 48e4e1b0075..d0f4d02f146 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -20,9 +20,11 @@ package org.elasticsearch.index.shard; import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.store.LockObtainFailedException; +import org.apache.lucene.util.IOUtils; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.stats.IndexStats; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; @@ -30,6 +32,7 @@ import org.elasticsearch.cluster.routing.TestShardRouting; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.env.ShardLock; import org.elasticsearch.index.IndexService; @@ -48,12 +51,14 @@ import org.junit.Test; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ExecutionException; import static org.elasticsearch.cluster.metadata.IndexMetaData.*; import static org.elasticsearch.common.settings.Settings.settingsBuilder; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; @@ -432,4 +437,89 @@ public class IndexShardTests extends ESSingleNodeTestCase { response = client().prepareSearch("test").get(); assertHitCount(response, 0l); } + + public void testIndexDirIsDeletedWhenShardRemoved() throws Exception { + Environment env = getInstanceFromNode(Environment.class); + Path idxPath = env.sharedDataFile().resolve(randomAsciiOfLength(10)); + logger.info("--> idxPath: [{}]", idxPath); + Settings idxSettings = Settings.builder() + .put(IndexMetaData.SETTING_DATA_PATH, idxPath) + .build(); + createIndex("test", idxSettings); + ensureGreen("test"); + client().prepareIndex("test", "bar", "1").setSource("{}").setRefresh(true).get(); + SearchResponse response = client().prepareSearch("test").get(); + assertHitCount(response, 1l); + client().admin().indices().prepareDelete("test").get(); + assertPathHasBeenCleared(idxPath); + } + + public void testIndexCanChangeCustomDataPath() throws Exception { + Environment env = getInstanceFromNode(Environment.class); + Path idxPath = env.sharedDataFile().resolve(randomAsciiOfLength(10)); + final String INDEX = "idx"; + Path startDir = idxPath.resolve("start-" + randomAsciiOfLength(10)); + Path endDir = idxPath.resolve("end-" + randomAsciiOfLength(10)); + logger.info("--> start dir: [{}]", startDir.toAbsolutePath().toString()); + logger.info("--> end dir: [{}]", endDir.toAbsolutePath().toString()); + // temp dirs are automatically created, but the end dir is what + // startDir is going to be renamed as, so it needs to be deleted + // otherwise we get all sorts of errors about the directory + // already existing + IOUtils.rm(endDir); + + Settings sb = Settings.builder() + .put(IndexMetaData.SETTING_DATA_PATH, startDir.toAbsolutePath().toString()) + .build(); + Settings sb2 = Settings.builder() + .put(IndexMetaData.SETTING_DATA_PATH, endDir.toAbsolutePath().toString()) + .build(); + + logger.info("--> creating an index with data_path [{}]", startDir.toAbsolutePath().toString()); + createIndex(INDEX, sb); + ensureGreen(INDEX); + client().prepareIndex(INDEX, "bar", "1").setSource("{}").setRefresh(true).get(); + + SearchResponse resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); + assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); + + logger.info("--> closing the index [{}]", INDEX); + client().admin().indices().prepareClose(INDEX).get(); + logger.info("--> index closed, re-opening..."); + client().admin().indices().prepareOpen(INDEX).get(); + logger.info("--> index re-opened"); + ensureGreen(INDEX); + + resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); + assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); + + // Now, try closing and changing the settings + + logger.info("--> closing the index [{}]", INDEX); + client().admin().indices().prepareClose(INDEX).get(); + + logger.info("--> moving data on disk [{}] to [{}]", startDir.getFileName(), endDir.getFileName()); + assert Files.exists(endDir) == false : "end directory should not exist!"; + Files.move(startDir, endDir, StandardCopyOption.REPLACE_EXISTING); + + logger.info("--> updating settings..."); + client().admin().indices().prepareUpdateSettings(INDEX) + .setSettings(sb2) + .setIndicesOptions(IndicesOptions.fromOptions(true, false, true, true)) + .get(); + + assert Files.exists(startDir) == false : "start dir shouldn't exist"; + + logger.info("--> settings updated and files moved, re-opening index"); + client().admin().indices().prepareOpen(INDEX).get(); + logger.info("--> index re-opened"); + ensureGreen(INDEX); + + resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); + assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); + + assertAcked(client().admin().indices().prepareDelete(INDEX)); + assertPathHasBeenCleared(startDir.toAbsolutePath().toString()); + assertPathHasBeenCleared(endDir.toAbsolutePath().toString()); + } } diff --git a/core/src/test/java/org/elasticsearch/indices/IndicesCustomDataPathIT.java b/core/src/test/java/org/elasticsearch/indices/IndicesCustomDataPathIT.java deleted file mode 100644 index 46d6f126f81..00000000000 --- a/core/src/test/java/org/elasticsearch/indices/IndicesCustomDataPathIT.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.indices; - -import org.apache.lucene.util.IOUtils; -import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.cluster.metadata.IndexMetaData; -import org.elasticsearch.common.io.PathUtils; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.test.junit.annotations.TestLogging; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardCopyOption; - -import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.equalTo; - -/** - * Tests for custom data path locations and templates - */ -@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0) -public class IndicesCustomDataPathIT extends ESIntegTestCase { - - private String path; - - private Settings nodeSettings(Path dataPath) { - return nodeSettings(dataPath.toString()); - } - - private Settings nodeSettings(String dataPath) { - return Settings.builder() - .put("node.add_id_to_custom_path", false) - .put("path.shared_data", dataPath) - .put("index.store.fs.fs_lock", randomFrom("native", "simple")) - .build(); - } - - @Before - public void setup() { - path = createTempDir().toAbsolutePath().toString(); - } - - @After - public void teardown() throws Exception { - IOUtils.deleteFilesIgnoringExceptions(PathUtils.get(path)); - } - - @Test - @TestLogging("_root:DEBUG,index:TRACE") - @AwaitsFix(bugUrl = "path shenanigans, Lee is looking into it") - public void testDataPathCanBeChanged() throws Exception { - final String INDEX = "idx"; - Path root = createTempDir(); - internalCluster().startNodesAsync(1, nodeSettings(root)); - Path startDir = root.resolve("start"); - Path endDir = root.resolve("end"); - logger.info("--> start dir: [{}]", startDir.toAbsolutePath().toString()); - logger.info("--> end dir: [{}]", endDir.toAbsolutePath().toString()); - // temp dirs are automatically created, but the end dir is what - // startDir is going to be renamed as, so it needs to be deleted - // otherwise we get all sorts of errors about the directory - // already existing - IOUtils.rm(endDir); - - Settings.Builder sb = Settings.builder().put(IndexMetaData.SETTING_DATA_PATH, - startDir.toAbsolutePath().toString()); - Settings.Builder sb2 = Settings.builder().put(IndexMetaData.SETTING_DATA_PATH, - endDir.toAbsolutePath().toString()); - - logger.info("--> creating an index with data_path [{}]", startDir.toAbsolutePath().toString()); - client().admin().indices().prepareCreate(INDEX).setSettings(sb).get(); - ensureGreen(INDEX); - - indexRandom(true, client().prepareIndex(INDEX, "doc", "1").setSource("{\"body\": \"foo\"}")); - - SearchResponse resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); - assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); - - logger.info("--> closing the index [{}]", INDEX); - client().admin().indices().prepareClose(INDEX).get(); - logger.info("--> index closed, re-opening..."); - client().admin().indices().prepareOpen(INDEX).get(); - logger.info("--> index re-opened"); - ensureGreen(INDEX); - - resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); - assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); - - // Now, try closing and changing the settings - - logger.info("--> closing the index [{}]", INDEX); - client().admin().indices().prepareClose(INDEX).get(); - - logger.info("--> moving data on disk [{}] to [{}]", startDir.getFileName(), endDir.getFileName()); - assert Files.exists(endDir) == false : "end directory should not exist!"; - Files.move(startDir, endDir, StandardCopyOption.REPLACE_EXISTING); - - logger.info("--> updating settings..."); - client().admin().indices().prepareUpdateSettings(INDEX) - .setSettings(sb2) - .setIndicesOptions(IndicesOptions.fromOptions(true, false, true, true)) - .get(); - - assert Files.exists(startDir) == false : "start dir shouldn't exist"; - - logger.info("--> settings updated and files moved, re-opening index"); - client().admin().indices().prepareOpen(INDEX).get(); - logger.info("--> index re-opened"); - ensureGreen(INDEX); - - resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); - assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); - - assertAcked(client().admin().indices().prepareDelete(INDEX)); - assertPathHasBeenCleared(startDir.toAbsolutePath().toString()); - assertPathHasBeenCleared(endDir.toAbsolutePath().toString()); - } - - @Test - @AwaitsFix(bugUrl = "path shenanigans, Lee is looking into it") - public void testIndexCreatedWithCustomPathAndTemplate() throws Exception { - final String INDEX = "myindex2"; - internalCluster().startNodesAsync(1, nodeSettings(path)); - - logger.info("--> creating an index with data_path [{}]", path); - Settings.Builder sb = Settings.builder() - .put(IndexMetaData.SETTING_DATA_PATH, path) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0); - - client().admin().indices().prepareCreate(INDEX).setSettings(sb).get(); - ensureGreen(INDEX); - - indexRandom(true, client().prepareIndex(INDEX, "doc", "1").setSource("{\"body\": \"foo\"}")); - - SearchResponse resp = client().prepareSearch(INDEX).setQuery(matchAllQuery()).get(); - assertThat("found the hit", resp.getHits().getTotalHits(), equalTo(1L)); - assertAcked(client().admin().indices().prepareDelete(INDEX)); - assertPathHasBeenCleared(path); - } -} diff --git a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java index 7e8b0830a3d..4bb9b7b7b60 100644 --- a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1808,39 +1808,6 @@ public abstract class ESIntegTestCase extends ESTestCase { return nodes; } - /** - * Asserts that there are no files in the specified path - */ - public void assertPathHasBeenCleared(String path) throws Exception { - assertPathHasBeenCleared(PathUtils.get(path)); - } - - /** - * Asserts that there are no files in the specified path - */ - public void assertPathHasBeenCleared(Path path) throws Exception { - logger.info("--> checking that [{}] has been cleared", path); - int count = 0; - StringBuilder sb = new StringBuilder(); - sb.append("["); - if (Files.exists(path)) { - try (DirectoryStream stream = Files.newDirectoryStream(path)) { - for (Path file : stream) { - logger.info("--> found file: [{}]", file.toAbsolutePath().toString()); - if (Files.isDirectory(file)) { - assertPathHasBeenCleared(file); - } else if (Files.isRegularFile(file)) { - count++; - sb.append(file.toAbsolutePath().toString()); - sb.append("\n"); - } - } - } - } - sb.append("]"); - assertThat(count + " files exist that should have been cleaned:\n" + sb.toString(), count, equalTo(0)); - } - protected static class NumShards { public final int numPrimaries; public final int numReplicas; diff --git a/core/src/test/java/org/elasticsearch/test/ESTestCase.java b/core/src/test/java/org/elasticsearch/test/ESTestCase.java index 1694ecf2e4f..e172ea2cde4 100644 --- a/core/src/test/java/org/elasticsearch/test/ESTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESTestCase.java @@ -64,7 +64,9 @@ import org.junit.rules.RuleChain; import java.io.IOException; import java.lang.reflect.Field; +import java.nio.file.DirectoryStream; import java.nio.file.FileSystem; +import java.nio.file.Files; import java.nio.file.Path; import java.util.*; import java.util.concurrent.Callable; @@ -73,6 +75,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import static com.google.common.collect.Lists.newArrayList; +import static org.hamcrest.Matchers.equalTo; /** * Base testcase for randomized unit testing with Elasticsearch @@ -576,4 +579,36 @@ public abstract class ESTestCase extends LuceneTestCase { return enabled; } + /** + * Asserts that there are no files in the specified path + */ + public void assertPathHasBeenCleared(String path) throws Exception { + assertPathHasBeenCleared(PathUtils.get(path)); + } + + /** + * Asserts that there are no files in the specified path + */ + public void assertPathHasBeenCleared(Path path) throws Exception { + logger.info("--> checking that [{}] has been cleared", path); + int count = 0; + StringBuilder sb = new StringBuilder(); + sb.append("["); + if (Files.exists(path)) { + try (DirectoryStream stream = Files.newDirectoryStream(path)) { + for (Path file : stream) { + logger.info("--> found file: [{}]", file.toAbsolutePath().toString()); + if (Files.isDirectory(file)) { + assertPathHasBeenCleared(file); + } else if (Files.isRegularFile(file)) { + count++; + sb.append(file.toAbsolutePath().toString()); + sb.append("\n"); + } + } + } + } + sb.append("]"); + assertThat(count + " files exist that should have been cleaned:\n" + sb.toString(), count, equalTo(0)); + } }