Skip to content
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

[TEST] More MetadataStateFormat tests #78577

Merged
merged 17 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,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);
Expand All @@ -362,7 +362,7 @@ private long findMaxGenerationId(final String prefix, Path... locations) throws
return maxId;
}

private List<Path> findStateFilesByGeneration(final long generation, Path... locations) {
List<Path> findStateFilesByGeneration(final long generation, Path... locations) {
List<Path> files = new ArrayList<>();
if (generation == -1) {
return files;
Expand Down Expand Up @@ -430,6 +430,9 @@ public Tuple<T, Long> 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 [" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.apache.lucene.store.MockDirectoryWrapper;
import org.apache.lucene.store.NIOFSDirectory;
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;
Expand All @@ -34,6 +35,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;

Expand Down Expand Up @@ -344,6 +346,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));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can also verify that we can find max genId and read state when supplying all paths (and get the new state out)?


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));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also verify that format.loadLatestStage(..., paths) return null and that findMaxGenerationId return -1?


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<Path> 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<DummyState> {
private enum FailureMode {
NO_FAILURES,
Expand All @@ -353,6 +470,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";
Expand All @@ -361,6 +479,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.
Expand Down Expand Up @@ -392,21 +511,37 @@ 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);
if (failureMode == FailureMode.FAIL_ON_METHOD) {
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);
}
}
}
Expand All @@ -415,9 +550,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);
}
}
};
Expand Down