Skip to content

Commit a81fbb9

Browse files
authored
Run CheckIndex on metadata index before loading (#73239)
The metadata index is small and important and only read at startup. Today we rely on Lucene to spot if any of its components is corrupt, but Lucene does not necesssarily verify all checksums in order to catch a corruption. With this commit we run `CheckIndex` on the metadata index first, and fail on startup if a corruption is detected. Closes #29358
1 parent 7d66561 commit a81fbb9

File tree

3 files changed

+77
-11
lines changed

3 files changed

+77
-11
lines changed

server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.apache.lucene.document.Field;
1717
import org.apache.lucene.document.StoredField;
1818
import org.apache.lucene.document.StringField;
19+
import org.apache.lucene.index.CheckIndex;
1920
import org.apache.lucene.index.DirectoryReader;
2021
import org.apache.lucene.index.IndexNotFoundException;
2122
import org.apache.lucene.index.IndexWriter;
@@ -46,6 +47,7 @@
4647
import org.elasticsearch.common.bytes.BytesReference;
4748
import org.elasticsearch.common.bytes.RecyclingBytesStreamOutput;
4849
import org.elasticsearch.common.io.Streams;
50+
import org.elasticsearch.common.io.stream.BytesStreamOutput;
4951
import org.elasticsearch.core.Releasable;
5052
import org.elasticsearch.core.Releasables;
5153
import org.elasticsearch.common.logging.Loggers;
@@ -70,6 +72,8 @@
7072
import java.io.Closeable;
7173
import java.io.IOError;
7274
import java.io.IOException;
75+
import java.io.PrintStream;
76+
import java.nio.charset.StandardCharsets;
7377
import java.nio.file.Files;
7478
import java.nio.file.Path;
7579
import java.util.ArrayList;
@@ -290,17 +294,46 @@ public static void overrideVersion(Version newVersion, Path... dataPaths) throws
290294
* Loads the available on-disk cluster state. Returns {@link OnDiskState#NO_ON_DISK_STATE} if no such state was found.
291295
*/
292296
public OnDiskState loadOnDiskState() throws IOException {
297+
return loadOnDiskState(true);
298+
}
299+
300+
/**
301+
* Loads the available on-disk cluster state. Returns {@link OnDiskState#NO_ON_DISK_STATE} if no such state was found.
302+
* @param checkClean whether to check the index for corruption before loading, only for tests
303+
*/
304+
OnDiskState loadOnDiskState(boolean checkClean) throws IOException {
293305
OnDiskState onDiskState = OnDiskState.NO_ON_DISK_STATE;
294306

295307
final Path indexPath = dataPath.resolve(METADATA_DIRECTORY_NAME);
296308
if (Files.exists(indexPath)) {
297-
try (Directory directory = createDirectory(indexPath);
298-
DirectoryReader directoryReader = DirectoryReader.open(directory)) {
299-
onDiskState = loadOnDiskState(dataPath, directoryReader);
309+
try (Directory directory = createDirectory(indexPath)) {
310+
if (checkClean) {
311+
try (BytesStreamOutput outputStream = new BytesStreamOutput()) {
312+
final boolean isClean;
313+
try (PrintStream printStream = new PrintStream(outputStream, true, StandardCharsets.UTF_8);
314+
CheckIndex checkIndex = new CheckIndex(directory)) {
315+
checkIndex.setInfoStream(printStream);
316+
checkIndex.setChecksumsOnly(true);
317+
isClean = checkIndex.checkIndex().clean;
318+
}
300319

301-
if (nodeId.equals(onDiskState.nodeId) == false) {
302-
throw new IllegalStateException("unexpected node ID in metadata, found [" + onDiskState.nodeId +
303-
"] in [" + dataPath + "] but expected [" + nodeId + "]");
320+
if (isClean == false) {
321+
if (logger.isErrorEnabled()) {
322+
outputStream.bytes().utf8ToString().lines().forEach(l -> logger.error("checkIndex: {}", l));
323+
}
324+
throw new IllegalStateException("the index containing the cluster metadata under the data path [" + dataPath +
325+
"] has been changed by an external force after it was last written by Elasticsearch and is now unreadable");
326+
}
327+
}
328+
}
329+
330+
try (DirectoryReader directoryReader = DirectoryReader.open(directory)) {
331+
onDiskState = loadOnDiskState(dataPath, directoryReader);
332+
333+
if (nodeId.equals(onDiskState.nodeId) == false) {
334+
throw new IllegalStateException("the index containing the cluster metadata under the data path [" + dataPath +
335+
"] belongs to a node with ID [" + onDiskState.nodeId + "] but this node's ID is [" + nodeId + "]");
336+
}
304337
}
305338
} catch (IndexNotFoundException e) {
306339
logger.debug(new ParameterizedMessage("no on-disk state at {}", indexPath), e);

server/src/test/java/org/elasticsearch/gateway/GatewayMetaStatePersistedStateTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ public void testDataOnlyNodePersistence() throws Exception {
370370
assertThat(persistedState.getLastAcceptedState().getLastAcceptedConfiguration(),
371371
not(equalTo(persistedState.getLastAcceptedState().getLastCommittedConfiguration())));
372372
CoordinationMetadata persistedCoordinationMetadata =
373-
persistedClusterStateService.loadOnDiskState().metadata.coordinationMetadata();
373+
persistedClusterStateService.loadOnDiskState(false).metadata.coordinationMetadata();
374374
assertThat(persistedCoordinationMetadata.getLastAcceptedConfiguration(),
375375
equalTo(GatewayMetaState.AsyncPersistedState.staleStateConfiguration));
376376
assertThat(persistedCoordinationMetadata.getLastCommittedConfiguration(),
@@ -386,12 +386,12 @@ public void testDataOnlyNodePersistence() throws Exception {
386386
.clusterUUID(state.metadata().clusterUUID()).clusterUUIDCommitted(true).build()).build();
387387

388388
assertClusterStateEqual(expectedClusterState, persistedState.getLastAcceptedState());
389-
persistedCoordinationMetadata = persistedClusterStateService.loadOnDiskState().metadata.coordinationMetadata();
389+
persistedCoordinationMetadata = persistedClusterStateService.loadOnDiskState(false).metadata.coordinationMetadata();
390390
assertThat(persistedCoordinationMetadata.getLastAcceptedConfiguration(),
391391
equalTo(GatewayMetaState.AsyncPersistedState.staleStateConfiguration));
392392
assertThat(persistedCoordinationMetadata.getLastCommittedConfiguration(),
393393
equalTo(GatewayMetaState.AsyncPersistedState.staleStateConfiguration));
394-
assertTrue(persistedClusterStateService.loadOnDiskState().metadata.clusterUUIDCommitted());
394+
assertTrue(persistedClusterStateService.loadOnDiskState(false).metadata.clusterUUIDCommitted());
395395

396396
// generate a series of updates and check if batching works
397397
final String indexName = randomAlphaOfLength(10);

server/src/test/java/org/elasticsearch/gateway/PersistedClusterStateServiceTests.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.apache.lucene.index.IndexWriter;
1414
import org.apache.lucene.index.IndexWriterConfig;
1515
import org.apache.lucene.index.Term;
16+
import org.apache.lucene.mockfile.ExtrasFS;
1617
import org.apache.lucene.store.Directory;
1718
import org.apache.lucene.store.FilterDirectory;
1819
import org.apache.lucene.store.IOContext;
@@ -38,24 +39,32 @@
3839
import org.elasticsearch.gateway.PersistedClusterStateService.Writer;
3940
import org.elasticsearch.index.Index;
4041
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
42+
import org.elasticsearch.test.CorruptionUtils;
4143
import org.elasticsearch.test.ESTestCase;
4244
import org.elasticsearch.test.MockLogAppender;
4345
import org.elasticsearch.test.junit.annotations.TestLogging;
4446

4547
import java.io.IOError;
4648
import java.io.IOException;
49+
import java.nio.file.DirectoryStream;
50+
import java.nio.file.Files;
4751
import java.nio.file.Path;
4852
import java.util.ArrayList;
4953
import java.util.Collection;
5054
import java.util.List;
5155
import java.util.concurrent.atomic.AtomicBoolean;
5256
import java.util.concurrent.atomic.AtomicLong;
57+
import java.util.stream.Collectors;
58+
import java.util.stream.StreamSupport;
5359

60+
import static org.apache.lucene.index.IndexWriter.WRITE_LOCK_NAME;
5461
import static org.hamcrest.Matchers.allOf;
5562
import static org.hamcrest.Matchers.containsString;
63+
import static org.hamcrest.Matchers.endsWith;
5664
import static org.hamcrest.Matchers.equalTo;
5765
import static org.hamcrest.Matchers.lessThan;
5866
import static org.hamcrest.Matchers.nullValue;
67+
import static org.hamcrest.Matchers.startsWith;
5968

6069
public class PersistedClusterStateServiceTests extends ESTestCase {
6170

@@ -73,7 +82,7 @@ public void testPersistsAndReloadsTerm() throws IOException {
7382
assertThat(persistedClusterStateService.loadOnDiskState().currentTerm, equalTo(0L));
7483
try (Writer writer = persistedClusterStateService.createWriter()) {
7584
writer.writeFullStateAndCommit(newTerm, ClusterState.EMPTY_STATE);
76-
assertThat(persistedClusterStateService.loadOnDiskState().currentTerm, equalTo(newTerm));
85+
assertThat(persistedClusterStateService.loadOnDiskState(false).currentTerm, equalTo(newTerm));
7786
}
7887

7988
assertThat(persistedClusterStateService.loadOnDiskState().currentTerm, equalTo(newTerm));
@@ -638,6 +647,30 @@ public void testSlowLogging() throws IOException, IllegalAccessException {
638647
}
639648
}
640649

650+
public void testFailsIfCorrupt() throws IOException {
651+
try (NodeEnvironment nodeEnvironment = newNodeEnvironment(createTempDir())) {
652+
final PersistedClusterStateService persistedClusterStateService = newPersistedClusterStateService(nodeEnvironment);
653+
654+
try (Writer writer = persistedClusterStateService.createWriter()) {
655+
writer.writeFullStateAndCommit(1, ClusterState.EMPTY_STATE);
656+
}
657+
658+
try (DirectoryStream<Path> directoryStream = Files.newDirectoryStream(nodeEnvironment.nodeDataPath().resolve("_state"))) {
659+
CorruptionUtils.corruptFile(random(), randomFrom(StreamSupport
660+
.stream(directoryStream.spliterator(), false)
661+
.filter(p -> {
662+
final String filename = p.getFileName().toString();
663+
return ExtrasFS.isExtra(filename) == false && filename.equals(WRITE_LOCK_NAME) == false;
664+
})
665+
.collect(Collectors.toList())));
666+
}
667+
668+
assertThat(expectThrows(IllegalStateException.class, persistedClusterStateService::loadOnDiskState).getMessage(), allOf(
669+
startsWith("the index containing the cluster metadata under the data path ["),
670+
endsWith("] has been changed by an external force after it was last written by Elasticsearch and is now unreadable")));
671+
}
672+
}
673+
641674
private void assertExpectedLogs(long currentTerm, ClusterState previousState, ClusterState clusterState,
642675
PersistedClusterStateService.Writer writer, MockLogAppender.LoggingExpectation expectation)
643676
throws IllegalAccessException, IOException {
@@ -675,7 +708,7 @@ private NodeEnvironment newNodeEnvironment(Path dataPath) throws IOException {
675708
}
676709

677710
private static ClusterState loadPersistedClusterState(PersistedClusterStateService persistedClusterStateService) throws IOException {
678-
final PersistedClusterStateService.OnDiskState onDiskState = persistedClusterStateService.loadOnDiskState();
711+
final PersistedClusterStateService.OnDiskState onDiskState = persistedClusterStateService.loadOnDiskState(false);
679712
return clusterStateFromMetadata(onDiskState.lastAcceptedVersion, onDiskState.metadata);
680713
}
681714

0 commit comments

Comments
 (0)