-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Add Repository Consistency Assertion to SnapshotResiliencyTests #40857
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
Changes from all commits
10bb32d
89d30bf
91f5e93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,6 +105,8 @@ | |
import org.elasticsearch.cluster.service.ClusterApplierService; | ||
import org.elasticsearch.cluster.service.ClusterService; | ||
import org.elasticsearch.cluster.service.MasterService; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.bytes.BytesArray; | ||
import org.elasticsearch.common.io.stream.NamedWriteableRegistry; | ||
import org.elasticsearch.common.network.NetworkModule; | ||
import org.elasticsearch.common.settings.ClusterSettings; | ||
|
@@ -115,7 +117,11 @@ | |
import org.elasticsearch.common.util.PageCacheRecycler; | ||
import org.elasticsearch.common.util.concurrent.AbstractRunnable; | ||
import org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor; | ||
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; | ||
import org.elasticsearch.common.xcontent.NamedXContentRegistry; | ||
import org.elasticsearch.common.xcontent.XContentHelper; | ||
import org.elasticsearch.common.xcontent.XContentParser; | ||
import org.elasticsearch.common.xcontent.XContentType; | ||
import org.elasticsearch.env.Environment; | ||
import org.elasticsearch.env.NodeEnvironment; | ||
import org.elasticsearch.env.TestEnvironment; | ||
|
@@ -140,8 +146,10 @@ | |
import org.elasticsearch.ingest.IngestService; | ||
import org.elasticsearch.node.ResponseCollectorService; | ||
import org.elasticsearch.plugins.PluginsService; | ||
import org.elasticsearch.repositories.IndexId; | ||
import org.elasticsearch.repositories.RepositoriesService; | ||
import org.elasticsearch.repositories.Repository; | ||
import org.elasticsearch.repositories.RepositoryData; | ||
import org.elasticsearch.repositories.fs.FsRepository; | ||
import org.elasticsearch.script.ScriptService; | ||
import org.elasticsearch.search.SearchService; | ||
|
@@ -160,6 +168,8 @@ | |
import org.junit.Before; | ||
|
||
import java.io.IOException; | ||
import java.nio.ByteBuffer; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.util.Collection; | ||
import java.util.Collections; | ||
|
@@ -206,8 +216,12 @@ public void createServices() { | |
} | ||
|
||
@After | ||
public void stopServices() { | ||
testClusterNodes.nodes.values().forEach(TestClusterNode::stop); | ||
public void verifyReposThenStopServices() throws IOException { | ||
try { | ||
assertNoStaleRepositoryData(); | ||
} finally { | ||
testClusterNodes.nodes.values().forEach(TestClusterNode::stop); | ||
} | ||
} | ||
|
||
public void testSuccessfulSnapshotAndRestore() { | ||
|
@@ -502,6 +516,65 @@ public void run() { | |
assertThat(snapshotIds, either(hasSize(1)).or(hasSize(0))); | ||
} | ||
|
||
/** | ||
* Assert that there are no unreferenced indices or unreferenced root-level metadata blobs in any repository. | ||
* TODO: Expand the logic here to also check for unreferenced segment blobs and shard level metadata | ||
*/ | ||
private void assertNoStaleRepositoryData() throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to split this method into separate methods and give them reasonable names, such as assertSnapshotUUIDs, assertIndexUUIDs, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure extracted those methods :) |
||
final Path repoPath = tempDir.resolve("repo").toAbsolutePath(); | ||
final List<Path> repos; | ||
try (Stream<Path> reposDir = Files.list(repoPath)) { | ||
repos = reposDir.filter(s -> s.getFileName().toString().startsWith("extra") == false).collect(Collectors.toList()); | ||
} | ||
for (Path repoRoot : repos) { | ||
final Path latestIndexGenBlob = repoRoot.resolve("index.latest"); | ||
assertTrue("Could not find index.latest blob for repo at [" + repoRoot + ']', Files.exists(latestIndexGenBlob)); | ||
final long latestGen = ByteBuffer.wrap(Files.readAllBytes(latestIndexGenBlob)).getLong(0); | ||
assertIndexGenerations(repoRoot, latestGen); | ||
final RepositoryData repositoryData; | ||
try (XContentParser parser = | ||
XContentHelper.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, | ||
new BytesArray(Files.readAllBytes(repoRoot.resolve("index-" + latestGen))), XContentType.JSON)) { | ||
repositoryData = RepositoryData.snapshotsFromXContent(parser, latestGen); | ||
} | ||
assertIndexUUIDs(repoRoot, repositoryData); | ||
assertSnapshotUUIDs(repoRoot, repositoryData); | ||
} | ||
} | ||
|
||
private static void assertIndexGenerations(Path repoRoot, long latestGen) throws IOException { | ||
try (Stream<Path> repoRootBlobs = Files.list(repoRoot)) { | ||
final long[] indexGenerations = repoRootBlobs.filter(p -> p.getFileName().toString().startsWith("index-")) | ||
.map(p -> p.getFileName().toString().replace("index-", "")) | ||
.mapToLong(Long::parseLong).sorted().toArray(); | ||
assertEquals(latestGen, indexGenerations[indexGenerations.length - 1]); | ||
assertTrue(indexGenerations.length <= 2); | ||
} | ||
} | ||
|
||
private static void assertIndexUUIDs(Path repoRoot, RepositoryData repositoryData) throws IOException { | ||
final List<String> expectedIndexUUIDs = | ||
repositoryData.getIndices().values().stream().map(IndexId::getId).collect(Collectors.toList()); | ||
try (Stream<Path> indexRoots = Files.list(repoRoot.resolve("indices"))) { | ||
final List<String> foundIndexUUIDs = indexRoots.filter(s -> s.getFileName().toString().startsWith("extra") == false) | ||
.map(p -> p.getFileName().toString()).collect(Collectors.toList()); | ||
assertThat(foundIndexUUIDs, containsInAnyOrder(expectedIndexUUIDs.toArray(Strings.EMPTY_ARRAY))); | ||
} | ||
} | ||
|
||
private static void assertSnapshotUUIDs(Path repoRoot, RepositoryData repositoryData) throws IOException { | ||
final List<String> expectedSnapshotUUIDs = | ||
repositoryData.getSnapshotIds().stream().map(SnapshotId::getUUID).collect(Collectors.toList()); | ||
for (String prefix : new String[]{"snap-", "meta-"}) { | ||
try (Stream<Path> repoRootBlobs = Files.list(repoRoot)) { | ||
final Collection<String> foundSnapshotUUIDs = repoRootBlobs.filter(p -> p.getFileName().toString().startsWith(prefix)) | ||
.map(p -> p.getFileName().toString().replace(prefix, "").replace(".dat", "")) | ||
.collect(Collectors.toSet()); | ||
assertThat(foundSnapshotUUIDs, containsInAnyOrder(expectedSnapshotUUIDs.toArray(Strings.EMPTY_ARRAY))); | ||
} | ||
} | ||
} | ||
|
||
private void clearDisruptionsAndAwaitSync() { | ||
testClusterNodes.clearNetworkDisruptions(); | ||
runUntil(() -> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have two
After
methods - the first one stopServices, the second one - assertNoStaleRepositoryData?Even if we can have only one after method it probably makes sense to rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed it, can't really use another after here since I need to validate consistency before closing the services since that deletes the files.