diff --git a/server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java b/server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java index c6f4fa1a0e515..8b7119c99f3aa 100644 --- a/server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java +++ b/server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java @@ -334,7 +334,7 @@ public void cleanupOldFiles(final long currentGeneration, Path[] locations) { * @return maximum id of state file or -1 if no such files are found * @throws IOException if IOException occurs */ - private long findMaxGenerationId(final String prefix, Path... locations) throws IOException { + long findMaxGenerationId(final String prefix, Path... locations) throws IOException { long maxId = -1; for (Path dataLocation : locations) { final Path resolve = dataLocation.resolve(STATE_DIR_NAME); @@ -353,7 +353,7 @@ private long findMaxGenerationId(final String prefix, Path... locations) throws return maxId; } - private List findStateFilesByGeneration(final long generation, Path... locations) { + List findStateFilesByGeneration(final long generation, Path... locations) { List files = new ArrayList<>(); if (generation == -1) { return files; @@ -421,6 +421,9 @@ public Tuple loadLatestStateWithGeneration(Logger logger, NamedXContent long generation = findMaxGenerationId(prefix, dataLocations); T state = loadGeneration(logger, namedXContentRegistry, generation, dataLocations); + // It may not be possible to get into this state, if there's a bad state file the above + // call will throw ElasticsearchException. If there are no state files, we won't find a + // generation. if (generation > -1 && state == null) { throw new IllegalStateException("unable to find state files with generation id " + generation + " returned by findMaxGenerationId function, in data folders [" + diff --git a/server/src/test/java/org/elasticsearch/gateway/MetadataStateFormatTests.java b/server/src/test/java/org/elasticsearch/gateway/MetadataStateFormatTests.java index b2dfa09911f27..2104105800a68 100644 --- a/server/src/test/java/org/elasticsearch/gateway/MetadataStateFormatTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/MetadataStateFormatTests.java @@ -15,6 +15,7 @@ import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.MockDirectoryWrapper; import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -33,6 +34,7 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.stream.StreamSupport; @@ -334,6 +336,121 @@ public void testFailRandomlyAndReadAnyState() throws IOException { writeAndReadStateSuccessfully(format, paths); } + public void testInconsistentMultiPathState() throws IOException { + Path paths[] = new Path[3]; + for (int i = 0; i < paths.length; i++) { + paths[i] = createTempDir(); + } + Format format = new Format("foo-"); + + DummyState state = new DummyState(randomRealisticUnicodeOfCodepointLengthBetween(1, 100), randomInt(), randomLong(), + randomDouble(), randomBoolean()); + // Call write without clean-up to simulate multi-write transaction. + long genId = format.write(state, paths); + assertEquals(state, format.loadLatestState(logger, NamedXContentRegistry.EMPTY, paths)); + ensureOnlyOneStateFile(paths); + + for (Path path : paths) { + assertEquals(genId, format.findMaxGenerationId("foo-", path)); + } + assertEquals(0, format.findStateFilesByGeneration(-1, paths).size()); + assertEquals(paths.length, format.findStateFilesByGeneration(genId, paths).size()); + + Path badPath = paths[paths.length-1]; + + format.failOnPaths(badPath.resolve(MetadataStateFormat.STATE_DIR_NAME)); + format.failOnMethods(Format.FAIL_RENAME_TMP_FILE); + + DummyState newState = new DummyState(randomRealisticUnicodeOfCodepointLengthBetween(1, 100), randomInt(), randomLong(), + randomDouble(), randomBoolean()); + + expectThrows(WriteStateException.class, () -> format.write(newState, paths)); + long firstPathId = format.findMaxGenerationId("foo-", paths[0]); + assertEquals(firstPathId, format.findMaxGenerationId("foo-", paths[1])); + assertEquals(genId, format.findMaxGenerationId("foo-", badPath)); + assertEquals(genId, firstPathId-1); + + // Since at least one path has the latest generation, we should find the latest + // generation when we supply all paths. + long allPathsId = format.findMaxGenerationId("foo-", paths); + assertEquals(firstPathId, allPathsId); + + // Assert that we can find the new state since one path successfully wrote it. + assertEquals(newState, format.loadLatestState(logger, NamedXContentRegistry.EMPTY, paths)); + } + + public void testDeleteMetaState() throws IOException { + Path paths[] = new Path[3]; + for (int i = 0; i < paths.length; i++) { + paths[i] = createTempDir(); + } + Format format = new Format("foo-"); + + DummyState state = new DummyState(randomRealisticUnicodeOfCodepointLengthBetween(1, 100), randomInt(), randomLong(), + randomDouble(), randomBoolean()); + long genId = format.write(state, paths); + assertEquals(state, format.loadLatestState(logger, NamedXContentRegistry.EMPTY, paths)); + ensureOnlyOneStateFile(paths); + + for (Path path : paths) { + assertEquals(genId, format.findMaxGenerationId("foo-", path)); + } + assertEquals(0, format.findStateFilesByGeneration(-1, paths).size()); + assertEquals(paths.length, format.findStateFilesByGeneration(genId, paths).size()); + + Format.deleteMetaState(paths); + + assertEquals(0, format.findStateFilesByGeneration(genId, paths).size()); + + for (Path path : paths) { + assertEquals(false, Files.exists(path.resolve(MetadataStateFormat.STATE_DIR_NAME))); + } + + // We shouldn't find any state or state generations anymore + assertEquals(-1, format.findMaxGenerationId("foo-", paths)); + assertNull(format.loadLatestState(logger, NamedXContentRegistry.EMPTY, paths)); + } + + public void testCleanupOldFilesWithErrorPath() throws IOException { + Path paths[] = new Path[3]; + for (int i = 0; i < paths.length; i++) { + paths[i] = createTempDir(); + } + Format format = new Format("foo-"); + + DummyState state = new DummyState(randomRealisticUnicodeOfCodepointLengthBetween(1, 100), randomInt(), randomLong(), + randomDouble(), randomBoolean()); + long genId = format.write(state, paths); + assertEquals(state, format.loadLatestState(logger, NamedXContentRegistry.EMPTY, paths)); + ensureOnlyOneStateFile(paths); + + for (Path path : paths) { + assertEquals(genId, format.findMaxGenerationId("foo-", path)); + } + assertEquals(0, format.findStateFilesByGeneration(-1, paths).size()); + assertEquals(paths.length, format.findStateFilesByGeneration(genId, paths).size()); + + List stateFiles = format.findStateFilesByGeneration(genId, paths); + + final int badDirIndex = 1; + + format.failOnPaths(paths[badDirIndex].resolve(MetadataStateFormat.STATE_DIR_NAME)); + format.failOnMethods(Format.FAIL_DELETE_TMP_FILE); + + // Ensure clean-up old files doesn't fail with one bad dir. We pretend we want to + // keep a newer generation that doesn't exist (genId + 1). + format.cleanupOldFiles(genId + 1, paths); + + // We simulated failure on deleting one stale state file, there should be one that's remaining from the old state. + // We'll corrupt this remaining file and check to see if loading the state throws an exception. + // All other state files, including the first directory uncorrupted state files should be cleaned up. + corruptFile(stateFiles.get(badDirIndex), logger); + + assertThat(expectThrows(ElasticsearchException.class, + () -> format.loadLatestStateWithGeneration(logger, xContentRegistry(), paths)).getMessage(), + equalTo("java.io.IOException: failed to read " + stateFiles.get(badDirIndex))); + } + private static class Format extends MetadataStateFormat { private enum FailureMode { NO_FAILURES, @@ -343,6 +460,7 @@ private enum FailureMode { private FailureMode failureMode; private String[] failureMethods; + private Path[] failurePaths; static final String FAIL_CREATE_OUTPUT_FILE = "createOutput"; static final String FAIL_WRITE_TO_OUTPUT_FILE = "writeBytes"; @@ -351,6 +469,7 @@ private enum FailureMode { static final String FAIL_FSYNC_STATE_DIRECTORY = "syncMetaData"; static final String FAIL_DELETE_TMP_FILE = "deleteFile"; static final String FAIL_OPEN_STATE_FILE_WHEN_COPYING = "openInput"; + static final String FAIL_LIST_ALL = "listAll"; /** * Constructs a MetadataStateFormat object for storing/retrieving DummyState. @@ -382,10 +501,26 @@ public void failOnMethods(String... failureMethods) { this.failureMethods = failureMethods; } + public void failOnPaths(Path... paths) { + this.failurePaths = paths; + } + public void failRandomly() { this.failureMode = FailureMode.FAIL_RANDOMLY; } + private void throwDirectoryExceptionCheckPaths(Path dir) throws MockDirectoryWrapper.FakeIOException { + if (failurePaths != null) { + for (Path p : failurePaths) { + if (p.equals(dir)) { + throw new MockDirectoryWrapper.FakeIOException(); + } + } + } else { + throw new MockDirectoryWrapper.FakeIOException(); + } + } + @Override protected Directory newDirectory(Path dir) { MockDirectoryWrapper mock = newMockFSDirectory(dir); @@ -393,10 +528,10 @@ protected Directory newDirectory(Path dir) { final String failMethod = randomFrom(failureMethods); MockDirectoryWrapper.Failure fail = new MockDirectoryWrapper.Failure() { @Override - public void eval(MockDirectoryWrapper dir) throws IOException { + public void eval(MockDirectoryWrapper directory) throws IOException { for (StackTraceElement e : Thread.currentThread().getStackTrace()) { if (failMethod.equals(e.getMethodName())) { - throw new MockDirectoryWrapper.FakeIOException(); + throwDirectoryExceptionCheckPaths(dir); } } } @@ -405,9 +540,9 @@ public void eval(MockDirectoryWrapper dir) throws IOException { } else if (failureMode == FailureMode.FAIL_RANDOMLY) { MockDirectoryWrapper.Failure fail = new MockDirectoryWrapper.Failure() { @Override - public void eval(MockDirectoryWrapper dir) throws IOException { + public void eval(MockDirectoryWrapper directory) throws IOException { if (randomIntBetween(0, 20) == 0) { - throw new MockDirectoryWrapper.FakeIOException(); + throwDirectoryExceptionCheckPaths(dir); } } };