From 8e5db90eec696110bb874e0ed0c0b2cff6412383 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 19 Dec 2018 07:39:10 +0100 Subject: [PATCH] Never corrupt fully deleted segments in tests (#36741) Today we might corrupt a fully deleted segment which is then pruned once a snapshot is taken. This causes random test failures in CorruptedFileIT. This change hardens the selection of files to corrupt and removes some fragile code preventing fully deleted segments to be taken into account. Closes #36526 --- .../index/store/CorruptedFileIT.java | 58 ++++++------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java b/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java index 8585c53d4f1..966495faa1e 100644 --- a/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java +++ b/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java @@ -21,7 +21,10 @@ package org.elasticsearch.index.store; import com.carrotsearch.hppc.cursors.IntObjectCursor; import com.carrotsearch.randomizedtesting.generators.RandomPicks; import org.apache.lucene.index.CheckIndex; -import org.apache.lucene.index.IndexFileNames; +import org.apache.lucene.index.SegmentCommitInfo; +import org.apache.lucene.index.SegmentInfos; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.cluster.node.stats.NodeStats; @@ -644,18 +647,26 @@ public class CorruptedFileIT extends ESIntegTestCase { Path file = PathUtils.get(path) .resolve("indices").resolve(test.getUUID()).resolve(Integer.toString(shardRouting.getId())).resolve("index"); if (Files.exists(file)) { // multi data path might only have one path in use - try (DirectoryStream stream = Files.newDirectoryStream(file)) { - for (Path item : stream) { - if (Files.isRegularFile(item) && "write.lock".equals(item.getFileName().toString()) == false) { - if (includePerCommitFiles || isPerSegmentFile(item.getFileName().toString())) { - files.add(item); + try (Directory dir = FSDirectory.open(file)) { + SegmentInfos segmentCommitInfos = Lucene.readSegmentInfos(dir); + if (includePerCommitFiles) { + files.add(file.resolve(segmentCommitInfos.getSegmentsFileName())); + } + for (SegmentCommitInfo commitInfo : segmentCommitInfos) { + if (commitInfo.getDelCount() + commitInfo.getSoftDelCount() == commitInfo.info.maxDoc()) { + // don't corrupt fully deleted segments - they might be removed on snapshot + continue; + } + for (String commitFile : commitInfo.files()) { + if (includePerCommitFiles || isPerSegmentFile(commitFile)) { + files.add(file.resolve(commitFile)); } } } + } } } - pruneOldDeleteGenerations(files); CorruptionUtils.corruptFile(random(), files.toArray(new Path[0])); return shardRouting; } @@ -669,39 +680,6 @@ public class CorruptedFileIT extends ESIntegTestCase { return isPerCommitFile(fileName) == false; } - /** - * prunes the list of index files such that only the latest del generation files are contained. - */ - private void pruneOldDeleteGenerations(Set files) { - final TreeSet delFiles = new TreeSet<>(); - for (Path file : files) { - if (file.getFileName().toString().endsWith(".liv") - || file.getFileName().toString().endsWith(".dvm") - || file.getFileName().toString().endsWith(".dvd") - ) { - delFiles.add(file); - } - } - Path last = null; - for (Path current : delFiles) { - if (last != null) { - final String newSegmentName = IndexFileNames.parseSegmentName(current.getFileName().toString()); - final String oldSegmentName = IndexFileNames.parseSegmentName(last.getFileName().toString()); - if (newSegmentName.equals(oldSegmentName) ) { - long oldGen = IndexFileNames.parseGeneration(last.getFileName().toString()); - long newGen = IndexFileNames.parseGeneration(current.getFileName().toString()); - if (newGen > oldGen) { - files.remove(last); - } else { - files.remove(current); - continue; - } - } - } - last = current; - } - } - public List listShardFiles(ShardRouting routing) throws IOException { NodesStatsResponse nodeStatses = client().admin().cluster().prepareNodesStats(routing.currentNodeId()).setFs(true).get(); ClusterState state = client().admin().cluster().prepareState().get().getState();