Skip to content

Commit dc44158

Browse files
DaveCTurnerDaveCTurner
committed
Remove support for ancient corrupted markers (#48858)
Today we still support reading store corruption markers of versions that haven't been written since 1.7. This commit removes this legacy support.
1 parent ab15bce commit dc44158

File tree

4 files changed

+40
-101
lines changed

4 files changed

+40
-101
lines changed

‎server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ public static boolean isCorruptMarkerFileIsPresent(final Directory directory) th
223223

224224
final String[] files = directory.listAll();
225225
for (String file : files) {
226-
if (file.startsWith(Store.CORRUPTED)) {
226+
if (file.startsWith(Store.CORRUPTED_MARKER_NAME_PREFIX)) {
227227
found = true;
228228
break;
229229
}
@@ -243,7 +243,7 @@ protected void dropCorruptMarkerFiles(Terminal terminal, Path path, Directory di
243243
String[] files = directory.listAll();
244244
boolean found = false;
245245
for (String file : files) {
246-
if (file.startsWith(Store.CORRUPTED)) {
246+
if (file.startsWith(Store.CORRUPTED_MARKER_NAME_PREFIX)) {
247247
directory.deleteFile(file);
248248

249249
terminal.println("Deleted corrupt marker " + file + " from " + path);

‎server/src/main/java/org/elasticsearch/index/store/Store.java‎

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,9 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref
138138
public static final Setting<Boolean> FORCE_RAM_TERM_DICT = Setting.boolSetting("index.force_memory_term_dictionary", false,
139139
Property.IndexScope, Property.Deprecated);
140140
static final String CODEC = "store";
141-
static final int VERSION_WRITE_THROWABLE= 2; // we write throwable since 2.0
142-
static final int VERSION_STACK_TRACE = 1; // we write the stack trace too since 1.4.0
143-
static final int VERSION_START = 0;
144-
static final int VERSION = VERSION_WRITE_THROWABLE;
141+
static final int CORRUPTED_MARKER_CODEC_VERSION = 2;
145142
// public is for test purposes
146-
public static final String CORRUPTED = "corrupted_";
143+
public static final String CORRUPTED_MARKER_NAME_PREFIX = "corrupted_";
147144
public static final Setting<TimeValue> INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING =
148145
Setting.timeSetting("index.store.stats_refresh_interval", TimeValue.timeValueSeconds(10), Property.IndexScope);
149146

@@ -448,7 +445,7 @@ public static MetadataSnapshot readMetadataSnapshot(Path indexLocation, ShardId
448445
Logger logger) throws IOException {
449446
try (ShardLock lock = shardLocker.lock(shardId, "read metadata snapshot", TimeUnit.SECONDS.toMillis(5));
450447
Directory dir = new SimpleFSDirectory(indexLocation)) {
451-
failIfCorrupted(dir, shardId);
448+
failIfCorrupted(dir);
452449
return new MetadataSnapshot(null, dir, logger);
453450
} catch (IndexNotFoundException ex) {
454451
// that's fine - happens all the time no need to log
@@ -469,7 +466,7 @@ public static void tryOpenIndex(Path indexLocation, ShardId shardId, NodeEnviron
469466
Logger logger) throws IOException, ShardLockObtainFailedException {
470467
try (ShardLock lock = shardLocker.lock(shardId, "open index", TimeUnit.SECONDS.toMillis(5));
471468
Directory dir = new SimpleFSDirectory(indexLocation)) {
472-
failIfCorrupted(dir, shardId);
469+
failIfCorrupted(dir);
473470
SegmentInfos segInfo = Lucene.readSegmentInfos(dir);
474471
logger.trace("{} loaded segment info [{}]", shardId, segInfo);
475472
}
@@ -550,7 +547,7 @@ public boolean isMarkedCorrupted() throws IOException {
550547
*/
551548
final String[] files = directory().listAll();
552549
for (String file : files) {
553-
if (file.startsWith(CORRUPTED)) {
550+
if (file.startsWith(CORRUPTED_MARKER_NAME_PREFIX)) {
554551
return true;
555552
}
556553
}
@@ -566,7 +563,7 @@ public void removeCorruptionMarker() throws IOException {
566563
IOException firstException = null;
567564
final String[] files = directory.listAll();
568565
for (String file : files) {
569-
if (file.startsWith(CORRUPTED)) {
566+
if (file.startsWith(CORRUPTED_MARKER_NAME_PREFIX)) {
570567
try {
571568
directory.deleteFile(file);
572569
} catch (IOException ex) {
@@ -585,40 +582,25 @@ public void removeCorruptionMarker() throws IOException {
585582

586583
public void failIfCorrupted() throws IOException {
587584
ensureOpen();
588-
failIfCorrupted(directory, shardId);
585+
failIfCorrupted(directory);
589586
}
590587

591-
private static void failIfCorrupted(Directory directory, ShardId shardId) throws IOException {
588+
private static void failIfCorrupted(Directory directory) throws IOException {
592589
final String[] files = directory.listAll();
593590
List<CorruptIndexException> ex = new ArrayList<>();
594591
for (String file : files) {
595-
if (file.startsWith(CORRUPTED)) {
592+
if (file.startsWith(CORRUPTED_MARKER_NAME_PREFIX)) {
596593
try (ChecksumIndexInput input = directory.openChecksumInput(file, IOContext.READONCE)) {
597-
int version = CodecUtil.checkHeader(input, CODEC, VERSION_START, VERSION);
598-
599-
if (version == VERSION_WRITE_THROWABLE) {
600-
final int size = input.readVInt();
601-
final byte[] buffer = new byte[size];
602-
input.readBytes(buffer, 0, buffer.length);
603-
StreamInput in = StreamInput.wrap(buffer);
604-
Exception t = in.readException();
605-
if (t instanceof CorruptIndexException) {
606-
ex.add((CorruptIndexException) t);
607-
} else {
608-
ex.add(new CorruptIndexException(t.getMessage(), "preexisting_corruption", t));
609-
}
594+
CodecUtil.checkHeader(input, CODEC, CORRUPTED_MARKER_CODEC_VERSION, CORRUPTED_MARKER_CODEC_VERSION);
595+
final int size = input.readVInt();
596+
final byte[] buffer = new byte[size];
597+
input.readBytes(buffer, 0, buffer.length);
598+
StreamInput in = StreamInput.wrap(buffer);
599+
Exception t = in.readException();
600+
if (t instanceof CorruptIndexException) {
601+
ex.add((CorruptIndexException) t);
610602
} else {
611-
assert version == VERSION_START || version == VERSION_STACK_TRACE;
612-
String msg = input.readString();
613-
StringBuilder builder = new StringBuilder(shardId.toString());
614-
builder.append(" Preexisting corrupted index [");
615-
builder.append(file).append("] caused by: ");
616-
builder.append(msg);
617-
if (version == VERSION_STACK_TRACE) {
618-
builder.append(System.lineSeparator());
619-
builder.append(input.readString());
620-
}
621-
ex.add(new CorruptIndexException(builder.toString(), "preexisting_corruption"));
603+
ex.add(new CorruptIndexException(t.getMessage(), "preexisting_corruption", t));
622604
}
623605
CodecUtil.checkFooter(input);
624606
}
@@ -654,7 +636,7 @@ public void cleanupAndVerify(String reason, MetadataSnapshot sourceMetaData) thr
654636
} catch (IOException ex) {
655637
if (existingFile.startsWith(IndexFileNames.SEGMENTS)
656638
|| existingFile.equals(IndexFileNames.OLD_SEGMENTS_GEN)
657-
|| existingFile.startsWith(CORRUPTED)) {
639+
|| existingFile.startsWith(CORRUPTED_MARKER_NAME_PREFIX)) {
658640
// TODO do we need to also fail this if we can't delete the pending commit file?
659641
// if one of those files can't be deleted we better fail the cleanup otherwise we might leave an old commit
660642
// point around?
@@ -1386,9 +1368,9 @@ public void deleteQuiet(String... files) {
13861368
public void markStoreCorrupted(IOException exception) throws IOException {
13871369
ensureOpen();
13881370
if (!isMarkedCorrupted()) {
1389-
String uuid = CORRUPTED + UUIDs.randomBase64UUID();
1390-
try (IndexOutput output = this.directory().createOutput(uuid, IOContext.DEFAULT)) {
1391-
CodecUtil.writeHeader(output, CODEC, VERSION);
1371+
final String corruptionMarkerName = CORRUPTED_MARKER_NAME_PREFIX + UUIDs.randomBase64UUID();
1372+
try (IndexOutput output = this.directory().createOutput(corruptionMarkerName, IOContext.DEFAULT)) {
1373+
CodecUtil.writeHeader(output, CODEC, CORRUPTED_MARKER_CODEC_VERSION);
13921374
BytesStreamOutput out = new BytesStreamOutput();
13931375
out.writeException(exception);
13941376
BytesReference bytes = out.bytes();
@@ -1399,7 +1381,7 @@ public void markStoreCorrupted(IOException exception) throws IOException {
13991381
} catch (IOException ex) {
14001382
logger.warn("Can't mark store as corrupted", ex);
14011383
}
1402-
directory().sync(Collections.singleton(uuid));
1384+
directory().sync(Collections.singleton(corruptionMarkerName));
14031385
}
14041386
}
14051387

‎server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3105,7 +3105,7 @@ public void testIndexCheckOnStartup() throws Exception {
31053105
final SimpleFileVisitor<Path> corruptedVisitor = new SimpleFileVisitor<Path>() {
31063106
@Override
31073107
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
3108-
if (Files.isRegularFile(file) && file.getFileName().toString().startsWith(Store.CORRUPTED)) {
3108+
if (Files.isRegularFile(file) && file.getFileName().toString().startsWith(Store.CORRUPTED_MARKER_NAME_PREFIX)) {
31093109
corruptedMarkerCount.incrementAndGet();
31103110
}
31113111
return FileVisitResult.CONTINUE;
@@ -3181,7 +3181,7 @@ public void testShardDoesNotStartIfCorruptedMarkerIsPresent() throws Exception {
31813181
final SimpleFileVisitor<Path> corruptedVisitor = new SimpleFileVisitor<Path>() {
31823182
@Override
31833183
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
3184-
if (Files.isRegularFile(file) && file.getFileName().toString().startsWith(Store.CORRUPTED)) {
3184+
if (Files.isRegularFile(file) && file.getFileName().toString().startsWith(Store.CORRUPTED_MARKER_NAME_PREFIX)) {
31853185
corruptedMarkerCount.incrementAndGet();
31863186
}
31873187
return FileVisitResult.CONTINUE;

‎server/src/test/java/org/elasticsearch/index/store/StoreTests.java‎

Lines changed: 13 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,17 @@
9292

9393
import static java.util.Collections.unmodifiableMap;
9494
import static org.elasticsearch.test.VersionUtils.randomVersion;
95+
import static org.hamcrest.Matchers.anyOf;
96+
import static org.hamcrest.Matchers.containsString;
9597
import static org.hamcrest.Matchers.empty;
9698
import static org.hamcrest.Matchers.endsWith;
9799
import static org.hamcrest.Matchers.equalTo;
98100
import static org.hamcrest.Matchers.greaterThan;
99101
import static org.hamcrest.Matchers.hasKey;
102+
import static org.hamcrest.Matchers.instanceOf;
100103
import static org.hamcrest.Matchers.is;
101104
import static org.hamcrest.Matchers.not;
102105
import static org.hamcrest.Matchers.notNullValue;
103-
import static org.hamcrest.Matchers.startsWith;
104106

105107
public class StoreTests extends ESTestCase {
106108

@@ -980,65 +982,20 @@ public void testDeserializeCorruptionException() throws IOException {
980982
store.close();
981983
}
982984

983-
public void testCanReadOldCorruptionMarker() throws IOException {
985+
public void testCorruptionMarkerVersionCheck() throws IOException {
984986
final ShardId shardId = new ShardId("index", "_na_", 1);
985987
final Directory dir = new RAMDirectory(); // I use ram dir to prevent that virusscanner being a PITA
986-
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
987-
988-
CorruptIndexException exception = new CorruptIndexException("foo", "bar");
989-
String uuid = Store.CORRUPTED + UUIDs.randomBase64UUID();
990-
try (IndexOutput output = dir.createOutput(uuid, IOContext.DEFAULT)) {
991-
CodecUtil.writeHeader(output, Store.CODEC, Store.VERSION_STACK_TRACE);
992-
output.writeString(exception.getMessage());
993-
output.writeString(ExceptionsHelper.stackTrace(exception));
994-
CodecUtil.writeFooter(output);
995-
}
996-
try {
997-
store.failIfCorrupted();
998-
fail("should be corrupted");
999-
} catch (CorruptIndexException e) {
1000-
assertThat(e.getMessage(), startsWith("[index][1] Preexisting corrupted index [" + uuid + "] caused by: foo (resource=bar)"));
1001-
assertTrue(e.getMessage().contains(ExceptionsHelper.stackTrace(exception)));
1002-
}
1003-
1004-
store.removeCorruptionMarker();
1005-
1006-
try (IndexOutput output = dir.createOutput(uuid, IOContext.DEFAULT)) {
1007-
CodecUtil.writeHeader(output, Store.CODEC, Store.VERSION_START);
1008-
output.writeString(exception.getMessage());
1009-
CodecUtil.writeFooter(output);
1010-
}
1011-
try {
1012-
store.failIfCorrupted();
1013-
fail("should be corrupted");
1014-
} catch (CorruptIndexException e) {
1015-
assertThat(e.getMessage(), startsWith("[index][1] Preexisting corrupted index [" + uuid + "] caused by: foo (resource=bar)"));
1016-
assertFalse(e.getMessage().contains(ExceptionsHelper.stackTrace(exception)));
1017-
}
1018988

1019-
store.removeCorruptionMarker();
1020-
1021-
try (IndexOutput output = dir.createOutput(uuid, IOContext.DEFAULT)) {
1022-
CodecUtil.writeHeader(output, Store.CODEC, Store.VERSION_START - 1); // corrupted header
1023-
CodecUtil.writeFooter(output);
1024-
}
1025-
try {
1026-
store.failIfCorrupted();
1027-
fail("should be too old");
1028-
} catch (IndexFormatTooOldException e) {
1029-
}
1030-
1031-
store.removeCorruptionMarker();
1032-
try (IndexOutput output = dir.createOutput(uuid, IOContext.DEFAULT)) {
1033-
CodecUtil.writeHeader(output, Store.CODEC, Store.VERSION+1); // corrupted header
1034-
CodecUtil.writeFooter(output);
1035-
}
1036-
try {
1037-
store.failIfCorrupted();
1038-
fail("should be too new");
1039-
} catch (IndexFormatTooNewException e) {
989+
try (Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId))) {
990+
final String corruptionMarkerName = Store.CORRUPTED_MARKER_NAME_PREFIX + UUIDs.randomBase64UUID();
991+
try (IndexOutput output = dir.createOutput(corruptionMarkerName, IOContext.DEFAULT)) {
992+
CodecUtil.writeHeader(output, Store.CODEC, Store.CORRUPTED_MARKER_CODEC_VERSION + randomFrom(1, 2, -1, -2, -3));
993+
// we only need the header to trigger the exception
994+
}
995+
final IOException ioException = expectThrows(IOException.class, store::failIfCorrupted);
996+
assertThat(ioException, anyOf(instanceOf(IndexFormatTooOldException.class), instanceOf(IndexFormatTooNewException.class)));
997+
assertThat(ioException.getMessage(), containsString(corruptionMarkerName));
1040998
}
1041-
store.close();
1042999
}
10431000

10441001
public void testEnsureIndexHasHistoryUUID() throws IOException {

0 commit comments

Comments
 (0)