diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java index 75f1fd780a..ad6bc00990 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java @@ -255,6 +255,11 @@ public class FDBRecordStore extends FDBStoreBase implements FDBRecordStoreBase preloadMetaData() { } } + /** + * In the event that a store has been corrupted, and the header has been lost, this method can be used to open + * the store, and fill in the missing header; replaced by {@link #repairMissingHeader(int, FormatVersion, boolean)}. + * + * @param userVersion the user version to set in the store header + * @param minimumPossibleFormatVersion the minimum {@link FormatVersion} that this store could have possibly + * had. Notably, upgrading to {@link FormatVersion#SAVE_VERSION_WITH_RECORD} requires moving data, and upgrading + * to {@link FormatVersion#SAVE_UNSPLIT_WITH_SUFFIX} requires storing whether the store should have the unsplit + * suffix or not. It is probably possible for {@code repairMissingHeader} to determine what to do based on the + * rest of the data in the store, but to keep this simple, upgrading across those versions is not supported. + * + * @return a boolean indicating whether a repair needed to be done ({@code true}) or not ({@code false}) and + * the opened store. + */ + @API(API.Status.DEPRECATED) + public CompletableFuture> repairMissingHeader(final int userVersion, FormatVersion minimumPossibleFormatVersion) { + return repairMissingHeader(userVersion, minimumPossibleFormatVersion, false); + } + /** * In the event that a store has been corrupted, and the header has been lost, this method can be used to open * the store, and fill in the missing header. @@ -5697,6 +5721,9 @@ private CompletableFuture preloadMetaData() { * Disable all indexes, since we cannot universally determine whether they have been added or * changed since the store was last opened. In theory there are some situations where we could * determine that the index can be marked readable for free, but this is not supported. + * If {@code leavePotentiallyCorruptIndexesReadable} is {@code true} they will not be disabled. + * This can be useful if you want to use the potentially corrupt indexes to discern information + * about the corrupted records. * *
  • * Disable the {@link RecordMetaData#getRecordCountKey()} if there is one on the metadata, because @@ -5727,12 +5754,16 @@ private CompletableFuture preloadMetaData() { * to {@link FormatVersion#SAVE_UNSPLIT_WITH_SUFFIX} requires storing whether the store should have the unsplit * suffix or not. It is probably possible for {@code repairMissingHeader} to determine what to do based on the * rest of the data in the store, but to keep this simple, upgrading across those versions is not supported. + * @param leavePotentiallyCorruptIndexesReadable if {@code true}, indexes will not be marked as disabled during + * repair, even though they could be corrupt. * * @return a boolean indicating whether a repair needed to be done ({@code true}) or not ({@code false}) and * the opened store. */ @API(API.Status.INTERNAL) - public CompletableFuture> repairMissingHeader(final int userVersion, FormatVersion minimumPossibleFormatVersion) { + public CompletableFuture> repairMissingHeader( + final int userVersion, @Nonnull final FormatVersion minimumPossibleFormatVersion, + final boolean leavePotentiallyCorruptIndexesReadable) { if (!formatVersion.isAtLeast(minimumPossibleFormatVersion)) { throw new RecordCoreArgumentException("minimumPossibleFormatVersion is greater than the target formatVerson") .addLogInfo(LogMessageKeys.FORMAT_VERSION, minimumPossibleFormatVersion) @@ -5748,12 +5779,13 @@ public CompletableFuture> repairMissingHead store.getStoreStateCache().get(store, StoreExistenceCheck.NONE) .thenCompose(storeStateCacheEntry -> repairMissingHeader(userVersion, store, - storeStateCacheEntry.getRecordStoreState().getStoreHeader()))); + storeStateCacheEntry.getRecordStoreState().getStoreHeader(), leavePotentiallyCorruptIndexesReadable))); } private CompletableFuture> repairMissingHeader(final int userVersion, - @Nonnull final FDBRecordStore store, - @Nonnull final RecordMetaDataProto.DataStoreInfo existing) { + @Nonnull final FDBRecordStore store, + @Nonnull final RecordMetaDataProto.DataStoreInfo existing, + boolean leavePotentiallyCorruptIndexesReadable) { if (!existing.equals(RecordMetaDataProto.DataStoreInfo.getDefaultInstance())) { return store.checkVersion(userVersionChecker, StoreExistenceCheck.ERROR_IF_NOT_EXISTS) .thenApply(checkVersionDidSomething -> NonnullPair.of(false, store)); @@ -5827,12 +5859,24 @@ private CompletableFuture> repairMissingHea // or that the store was on a metadata version that didn't have the index, or // had an older version of the index. If it wasn't readable on the current version, then // leaving the index would leave it in a corrupted state. - return bumpMetaDataVersionStamp.thenCompose(vignore -> AsyncUtil.whenAll( + return bumpMetaDataVersionStamp.thenCompose(vignore -> { + if (leavePotentiallyCorruptIndexesReadable) { + // Add a commit check that will fail if the user tries to commit with potentially corrupted indexes + store.getRecordContext().addCommitCheck(POTENTIALLY_CORRUPTED_INDEXES_COMMIT_CHECK, + new PreventCommitCheck(() -> new RecordCoreException( + "Commit failed because potentially corrupted indexes were left readable after header repair. " + + "The indexes should be rebuilt or verified before allowing commits."))); + // Leave indexes as-is per user request + return AsyncUtil.DONE; + } else { + return AsyncUtil.whenAll( recordMetaData.getAllIndexes().stream() .map(store::markIndexDisabled) - .collect(Collectors.toList())) - .thenApply(ignored -> NonnullPair.of(true, store))); + .collect(Collectors.toList())); + } + }).thenApply(ignored -> NonnullPair.of(true, store)); } + } } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/PreventCommitCheck.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/PreventCommitCheck.java new file mode 100644 index 0000000000..fd646c55d3 --- /dev/null +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/PreventCommitCheck.java @@ -0,0 +1,52 @@ +/* + * PreventCommitCheck.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.apple.foundationdb.record.provider.foundationdb; + +import com.apple.foundationdb.annotation.API; +import com.apple.foundationdb.record.RecordCoreException; + +import javax.annotation.Nonnull; +import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; + +/** + * Commit check that always fails, effectively preventing this transaction from committing at all. + */ +@API(API.Status.INTERNAL) +class PreventCommitCheck implements FDBRecordContext.CommitCheckAsync { + + final Supplier exceptionSupplier; + + PreventCommitCheck(Supplier exceptionSupplier) { + this.exceptionSupplier = exceptionSupplier; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + @Nonnull + public CompletableFuture checkAsync() { + return CompletableFuture.failedFuture(exceptionSupplier.get()); + } +} diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreRepairHeaderTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreRepairHeaderTest.java index 79827a9b28..38f256e650 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreRepairHeaderTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreRepairHeaderTest.java @@ -48,13 +48,13 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.ValueSource; import javax.annotation.Nonnull; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -77,13 +77,6 @@ static Stream formatVersions() { return Arrays.stream(FormatVersion.values()); } - static Stream repairUpgradeFormatVersion() { - return formatVersions() - .flatMap(oldVersion -> formatVersions().filter(newVersion -> newVersion.isAtLeast(oldVersion)) - .flatMap(newVersion -> Stream.of(true, false) - .map(supportSplitRecords -> Arguments.of(oldVersion, newVersion, supportSplitRecords)))); - } - @Test void checkMaxFormatVersion() { assertThat(FormatVersion.getMaximumSupportedVersion()) @@ -96,9 +89,19 @@ void checkMaxFormatVersion() { .isEqualTo(FormatVersion.STORE_LOCK_STATE); } + static Stream repairUpgradeFormatVersion() { + return formatVersions() + .flatMap(oldVersion -> formatVersions().filter(newVersion -> newVersion.isAtLeast(oldVersion)) + .flatMap(newVersion -> ParameterizedTestUtils.booleans("supportSplitRecords") + .flatMap(supportSplitRecords -> ParameterizedTestUtils.booleans("leaveIndexes") + .map(leavePotentiallyCorruptIndexesReadable -> + Arguments.of(oldVersion, newVersion, supportSplitRecords, leavePotentiallyCorruptIndexesReadable))))); + } + @ParameterizedTest @MethodSource - void repairUpgradeFormatVersion(FormatVersion oldFormatVersion, FormatVersion newFormatVersion, boolean supportSplitRecords) { + void repairUpgradeFormatVersion(FormatVersion oldFormatVersion, FormatVersion newFormatVersion, + boolean supportSplitRecords, boolean leavePotentiallyCorruptIndexesReadable) { final RecordMetaData recordMetaData = getRecordMetaData(supportSplitRecords); final List primaryKeys = createInitialStore(oldFormatVersion, recordMetaData); final List> originalRecords = createOriginalRecords(recordMetaData, primaryKeys); @@ -109,23 +112,30 @@ void repairUpgradeFormatVersion(FormatVersion oldFormatVersion, FormatVersion ne final FDBRecordStore.Builder builder = getStoreBuilder(context, recordMetaData) .setFormatVersion(newFormatVersion); if (oldFormatVersion.isAtLeast(FormatVersion.SAVE_VERSION_WITH_RECORD)) { - repairHeader(context, 1, builder, FormatVersion.SAVE_VERSION_WITH_RECORD); + assertThat(repairHeader(context, 1, builder, FormatVersion.SAVE_VERSION_WITH_RECORD, leavePotentiallyCorruptIndexesReadable)).isTrue(); } else if (!newFormatVersion.isAtLeast(FormatVersion.SAVE_UNSPLIT_WITH_SUFFIX)) { - repairHeader(context, 1, builder, FormatVersion.INFO_ADDED); + assertThat(repairHeader(context, 1, builder, FormatVersion.INFO_ADDED, leavePotentiallyCorruptIndexesReadable)).isTrue(); } else { - assertThatThrownBy(() -> repairHeader(context, 1, builder, oldFormatVersion)) + assertThatThrownBy(() -> repairHeader(context, 1, builder, oldFormatVersion, leavePotentiallyCorruptIndexesReadable)) .isInstanceOf(RecordCoreException.class); return; // nothing left to test } - commit(context); + + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } - validateRepaired(newFormatVersion, recordMetaData, originalRecords); + validateRepaired(newFormatVersion, recordMetaData, originalRecords, leavePotentiallyCorruptIndexesReadable); + } + + static Stream repairUserVersion() { + return ParameterizedTestUtils.cartesianProduct( + Stream.of(1, 3), + ParameterizedTestUtils.booleans("leaveIndexes")); } @ParameterizedTest - @ValueSource(ints = {1, 3}) - void repairUserVersion(int userVersion) { + @MethodSource + void repairUserVersion(int userVersion, boolean leavePotentiallyCorruptIndexesReadable) { final RecordMetaData recordMetaData = getRecordMetaData(true); final List primaryKeys = createInitialStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData); final List> originalRecords = createOriginalRecords(recordMetaData, primaryKeys); @@ -137,11 +147,12 @@ void repairUserVersion(int userVersion) { try (FDBRecordContext context = openContext()) { repairHeader(context, userVersion, getStoreBuilder(context, recordMetaData) .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) - .setUserVersionChecker(userVersionChecker)); - commit(context); + .setUserVersionChecker(userVersionChecker), leavePotentiallyCorruptIndexesReadable); + + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } - validateRepaired(userVersion, recordMetaData, userVersionChecker, originalRecords); + validateRepaired(userVersion, recordMetaData, userVersionChecker, originalRecords, leavePotentiallyCorruptIndexesReadable); } @Test @@ -167,12 +178,13 @@ void repairMetaDataVersion() { .setUserVersionChecker(new AssertMatchingMetaDataVersion(metadata2))); commit(context); } - validateRepaired(1, metadata2, new AssertMatchingMetaDataVersion(metadata2), originalRecords); + validateRepaired(1, metadata2, new AssertMatchingMetaDataVersion(metadata2), originalRecords, false); } - @Test - void needRebuildIndex() { + @ParameterizedTest + @BooleanSource("leavePotentiallyCorruptIndexesReadable") + void needRebuildIndex(boolean leavePotentiallyCorruptIndexesReadable) { final RecordMetaData recordMetaData = getRecordMetaData(true); final List primaryKeys = createInitialStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData); final List> originalRecords = createOriginalRecords(recordMetaData, primaryKeys); @@ -197,11 +209,12 @@ public IndexState needRebuildIndex(final Index index, final long recordCount, fi repairHeader(context, userVersion, getStoreBuilder(context, recordMetaData) .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) - .setUserVersionChecker(userVersionChecker)); - commit(context); + .setUserVersionChecker(userVersionChecker), leavePotentiallyCorruptIndexesReadable); + + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } - validateRepaired(userVersion, recordMetaData, userVersionChecker, originalRecords); + validateRepaired(userVersion, recordMetaData, userVersionChecker, originalRecords, leavePotentiallyCorruptIndexesReadable); } @Test @@ -223,17 +236,12 @@ void repairUserField() { commit(context); } - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) - .open(); - assertEquals(value, recordStore.getHeaderUserField(key)); - commit(context); - } + withStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData, + () -> assertEquals(value, recordStore.getHeaderUserField(key))); } @ParameterizedTest - @BooleanSource + @BooleanSource("doLock") void repairWithRecordsAndSetRecordUpdateLock(boolean doLock) { final RecordMetaData recordMetaData = getRecordMetaData(true); final List primaryKeys = createInitialStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData); @@ -253,19 +261,16 @@ void repairWithRecordsAndSetRecordUpdateLock(boolean doLock) { commit(context); } TestRecords1Proto.MySimpleRecord record = TestRecords1Proto.MySimpleRecord.newBuilder().setRecNo(200).setNumValue2(2100).build(); - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) - .open(); - if (doLock) { - // assert update record failure, then release - assertThrows(StoreIsLockedForRecordUpdates.class, () -> recordStore.saveRecord(record)); - recordStore.clearStoreLockStateAsync().join(); - } - // Successfully update a records after either not setting or releasing the records updates lock - recordStore.saveRecord(record); - commit(context); - } + withStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData, + () -> { + if (doLock) { + // assert update record failure, then release + assertThrows(StoreIsLockedForRecordUpdates.class, () -> recordStore.saveRecord(record)); + recordStore.clearStoreLockStateAsync().join(); + } + // Successfully update a records after either not setting or releasing the records updates lock + recordStore.saveRecord(record); + }); } @Test @@ -325,7 +330,7 @@ void repairCacheable() { } @ParameterizedTest - @BooleanSource + @BooleanSource("cached") void noRepairIfHeaderExists(boolean cached) { // We don't allow repair if it is in the database, or if it is cached. // In theory, we could use the cached value to inform what we fabricate, but the chance that you will discover @@ -356,13 +361,15 @@ static Stream disableRecordCountKeyOnRepair() { return ParameterizedTestUtils.cartesianProduct( ParameterizedTestUtils.booleans("hasRecordCountKey"), Stream.of(FormatVersion.getMaximumSupportedVersion(), - FormatVersionTestUtils.previous(FormatVersion.RECORD_COUNT_STATE))); + FormatVersionTestUtils.previous(FormatVersion.RECORD_COUNT_STATE)), + ParameterizedTestUtils.booleans("leavePotentiallyCorruptIndexesReadable")); } @ParameterizedTest @MethodSource @SuppressWarnings("deprecation") - void disableRecordCountKeyOnRepair(boolean hasRecordCountKey, FormatVersion formatVersion) { + void disableRecordCountKeyOnRepair(boolean hasRecordCountKey, FormatVersion formatVersion, + boolean leavePotentiallyCorruptIndexesReadable) { // We cannot tell whether the recordCountKey had changed since the last time we did checkVersion, so // we can't guarantee that it is correct. If there is a RecordCountKey and we're on a new enough format version // we'll disable the record count key, otherwise, we'll throw an exception @@ -383,52 +390,52 @@ void disableRecordCountKeyOnRepair(boolean hasRecordCountKey, FormatVersion form .setFormatVersion(formatVersion); if (hasRecordCountKey) { if (formatVersion.isAtLeast(FormatVersion.RECORD_COUNT_STATE)) { - repairHeader(context, userVersion, storeBuilder); + repairHeader(context, userVersion, storeBuilder, leavePotentiallyCorruptIndexesReadable); assertThat(recordStore.getRecordStoreState().getStoreHeader().getRecordCountState()) .isEqualTo(RecordMetaDataProto.DataStoreInfo.RecordCountState.DISABLED); - commit(context); + + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } else { - assertThatThrownBy(() -> repairHeader(context, userVersion, storeBuilder)) + assertThatThrownBy(() -> repairHeader(context, userVersion, storeBuilder, leavePotentiallyCorruptIndexesReadable)) .isInstanceOf(RecordCoreException.class); } } else { - repairHeader(context, userVersion, storeBuilder); - commit(context); + repairHeader(context, userVersion, storeBuilder, leavePotentiallyCorruptIndexesReadable); + + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } } - if (hasRecordCountKey) { - if (formatVersion.isAtLeast(FormatVersion.RECORD_COUNT_STATE)) { - validateRepaired(formatVersion, recordMetaData, originalRecords); - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(formatVersion) - .open(); - - assertThat(recordStore.getRecordStoreState().getStoreHeader().getRecordCountState()) - .isEqualTo(RecordMetaDataProto.DataStoreInfo.RecordCountState.DISABLED); - assertThat(recordStore.getRecordStoreState().getStoreHeader().getRecordCountKey()) - .isEqualTo(recordMetaData.getRecordCountKey().toKeyExpression()); + if (!leavePotentiallyCorruptIndexesReadable) { + if (hasRecordCountKey) { + if (formatVersion.isAtLeast(FormatVersion.RECORD_COUNT_STATE)) { + validateRepaired(formatVersion, recordMetaData, originalRecords, leavePotentiallyCorruptIndexesReadable); + validateRecordCountKeyIsDisabled(formatVersion, recordMetaData); + } else { + validateCannotOpen(recordMetaData); } } else { - validateCannotOpen(recordMetaData); + validateRepaired(formatVersion, recordMetaData, originalRecords, leavePotentiallyCorruptIndexesReadable); + withStore(formatVersion, recordMetaData, + () -> assertThat(recordStore.getRecordStoreState().getStoreHeader().hasRecordCountKey()).isFalse()); } } else { - validateRepaired(formatVersion, recordMetaData, originalRecords); - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(formatVersion) - .open(); - - assertThat(recordStore.getRecordStoreState().getStoreHeader().hasRecordCountKey()) - .isFalse(); - } + validateCannotOpen(recordMetaData); } } + private void validateRecordCountKeyIsDisabled(final FormatVersion formatVersion, final RecordMetaData recordMetaData) { + withStore(formatVersion, recordMetaData, () -> { + assertThat(recordStore.getRecordStoreState().getStoreHeader().getRecordCountState()) + .isEqualTo(RecordMetaDataProto.DataStoreInfo.RecordCountState.DISABLED); + assertThat(recordStore.getRecordStoreState().getStoreHeader().getRecordCountKey()) + .isEqualTo(recordMetaData.getRecordCountKey().toKeyExpression()); + }); + } + @ParameterizedTest - @BooleanSource - void clearAllFormerIndexes(final boolean removeBefore) { + @BooleanSource({"removeBefore", "leavePotentiallyCorruptIndexesReadable"}) + void clearAllFormerIndexes(final boolean removeBefore, boolean leavePotentiallyCorruptIndexesReadable) { RecordMetaDataBuilder metaDataBuilder = RecordMetaData.newBuilder().setRecords(TestRecords1Proto.getDescriptor()); metaDataBuilder.addIndex("MySimpleRecord", "ToRemove1", "num_value_2"); metaDataBuilder.addIndex("MySimpleRecord", "ToRemove2", "num_value_unique"); @@ -458,22 +465,25 @@ void clearAllFormerIndexes(final boolean removeBefore) { try (FDBRecordContext context = openContext()) { final FDBRecordStore.Builder builder = getStoreBuilder(context, recordMetaData); - repairHeader(context, 1, builder); - commit(context); - } + repairHeader(context, 1, builder, leavePotentiallyCorruptIndexesReadable); - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .open(); - final List indexes = recordStore.getAllIndexStates().keySet().stream() - .map(Index::getName).collect(Collectors.toList()); - assertThat(indexes).doesNotContain("ToRemove1", "ToRemove2"); - commit(context); + commitRepair(leavePotentiallyCorruptIndexesReadable, context, originalRecords); } - rawAssertHasIndexData(recordMetaData, false, subspaceKeys); + if (!leavePotentiallyCorruptIndexesReadable) { + try (FDBRecordContext context = openContext()) { + recordStore = getStoreBuilder(context, recordMetaData, path) + .open(); + final List indexes = recordStore.getAllIndexStates().keySet().stream() + .map(Index::getName).collect(Collectors.toList()); + assertThat(indexes).doesNotContain("ToRemove1", "ToRemove2"); + commit(context); + } + + rawAssertHasIndexData(recordMetaData, false, subspaceKeys); - validateRepaired(FormatVersion.getMaximumSupportedVersion(), recordMetaData, originalRecords); + validateRepaired(FormatVersion.getMaximumSupportedVersion(), recordMetaData, originalRecords, leavePotentiallyCorruptIndexesReadable); + } } /** @@ -539,6 +549,41 @@ void noStore() { } } + @ParameterizedTest + @BooleanSource("leavePotentiallyCorruptIndexesReadable") + void repairWithCommitChecks(boolean leavePotentiallyCorruptIndexesReadable) { + final RecordMetaData recordMetaData = getRecordMetaData(true); + final List primaryKeys = createInitialStore(FormatVersion.getMaximumSupportedVersion(), recordMetaData); + createOriginalRecords(recordMetaData, primaryKeys); + clearStoreHeader(recordMetaData); + validateCannotOpen(recordMetaData); + + try (FDBRecordContext context = openContext()) { + final FDBRecordStore.Builder builder = getStoreBuilder(context, recordMetaData) + .setFormatVersion(FormatVersion.getMaximumSupportedVersion()); + repairHeader(context, 1, builder, FormatVersion.CACHEABLE_STATE, leavePotentiallyCorruptIndexesReadable); + + if (leavePotentiallyCorruptIndexesReadable) { + // Verify the commit check exists + assertThat(context.getCommitCheck(FDBRecordStore.POTENTIALLY_CORRUPTED_INDEXES_COMMIT_CHECK)).isNotNull(); + + // Attempting to commit should fail with the expected exception + CompletionException completionException = assertThrows(CompletionException.class, () -> { + context.commitAsync().join(); + }); + assertThat(completionException.getCause()).isInstanceOf(RecordCoreException.class); + assertThat(completionException.getCause().getMessage()) + .contains("Commit failed because potentially corrupted indexes were left readable after header repair"); + } else { + // No commit check should be added when leavePotentiallyCorruptIndexesReadable is false + assertThat(context.getCommitCheck(FDBRecordStore.POTENTIALLY_CORRUPTED_INDEXES_COMMIT_CHECK)).isNull(); + + // Commit should succeed normally + commit(context); + } + } + } + private List> createOriginalRecords(final RecordMetaData recordMetaData, final List primaryKeys) { try (FDBRecordContext context = openContext()) { recordStore = getStoreBuilder(context, recordMetaData, path).open(); @@ -548,30 +593,49 @@ private List> createOriginalRecords(final RecordMetaDat } } - private void validateRepaired(final int userVersion, final RecordMetaData recordMetaData, - final FDBRecordStoreBase.UserVersionChecker userVersionChecker, - final List> originalRecords) { - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) - .setUserVersionChecker(userVersionChecker) - .open(); - assertEquals(userVersion, recordStore.getUserVersion()); + private void commitRepair(final boolean leavePotentiallyCorruptIndexesReadable, final FDBRecordContext context, final List> originalRecords) { + if (leavePotentiallyCorruptIndexesReadable) { + // Verify the commit check exists but don't attempt to commit validateRecords(originalRecords); - validateIndexesDisabled(recordMetaData); + assertThat(context.getCommitCheck(FDBRecordStore.POTENTIALLY_CORRUPTED_INDEXES_COMMIT_CHECK)).isNotNull(); + } else { commit(context); } } + private void validateRepaired(final int userVersion, final RecordMetaData recordMetaData, + final FDBRecordStoreBase.UserVersionChecker userVersionChecker, + final List> originalRecords, + boolean leavePotentiallyCorruptIndexesReadable) { + if (leavePotentiallyCorruptIndexesReadable) { + // if we left the indexes readable, we won't be able to commit the repair + validateCannotOpen(recordMetaData); + } else { + try (FDBRecordContext context = openContext()) { + recordStore = getStoreBuilder(context, recordMetaData, path) + .setFormatVersion(FormatVersion.getMaximumSupportedVersion()) + .setUserVersionChecker(userVersionChecker) + .open(); + assertEquals(userVersion, recordStore.getUserVersion()); + validateRecords(originalRecords); + validateIndexesAreDisabled(recordMetaData); + commit(context); + } + } + } + private void validateRepaired(final FormatVersion newFormatVersion, final RecordMetaData recordMetaData, - final List> originalRecords) { - try (FDBRecordContext context = openContext()) { - recordStore = getStoreBuilder(context, recordMetaData, path) - .setFormatVersion(newFormatVersion) - .open(); - validateRecords(originalRecords); - validateIndexesDisabled(recordMetaData); - commit(context); + final List> originalRecords, + boolean leavePotentiallyCorruptIndexesReadable) { + if (leavePotentiallyCorruptIndexesReadable) { + // if we left the indexes readable, we won't be able to commit the repair + validateCannotOpen(recordMetaData); + } else { + withStore(newFormatVersion, recordMetaData, + () -> { + validateRecords(originalRecords); + validateIndexesAreDisabled(recordMetaData); + }); } } @@ -586,9 +650,12 @@ private void validateRecords(final List> records) { /** * It is possible that the old (lost) metadata version was after the record type was created, and before the * index was added, and thus the index needs to be rebuilt. We could optimize this if the record type was added - * in the same version as the index, but for now, we're just going to disable all indexes. + * in the same version as the index, but for now, we're just going to disable all indexes unless + * leavePotentiallyCorruptIndexesReadable is true. + * If leavePotentiallyCorruptIndexesReadable is false, all indexes should be disabled. + * If leavePotentiallyCorruptIndexesReadable is true, indexes should remain in their previous state. */ - private void validateIndexesDisabled(final RecordMetaData recordMetaData) { + private void validateIndexesAreDisabled(final RecordMetaData recordMetaData) { for (final Index index : recordMetaData.getAllIndexes()) { assertEquals(IndexState.DISABLED, recordStore.getIndexState(index)); } @@ -633,6 +700,18 @@ private RecordMetaData getRecordMetaData(final boolean supportSplitRecords) { return simpleMetaData(metadata -> metadata.setSplitLongRecords(supportSplitRecords)); } + private void withStore(final FormatVersion formatVersion, + final RecordMetaData recordMetaData, + final Runnable useStore) { + try (FDBRecordContext context = openContext()) { + recordStore = getStoreBuilder(context, recordMetaData, path) + .setFormatVersion(formatVersion) + .open(); + useStore.run(); + commit(context); + } + } + private void validateCannotOpen(final RecordMetaDataProvider metaData) { validateCannotOpen(metaData, RecordStoreNoInfoAndNotEmptyException.class); } @@ -656,13 +735,24 @@ private void clearStoreHeader(final RecordMetaDataProvider metaData) { private void repairHeader(final FDBRecordContext context, final int userVersion, final FDBRecordStore.Builder builder) { - assertThat(repairHeader(context, userVersion, builder, FormatVersion.SAVE_VERSION_WITH_RECORD)).isTrue(); + assertThat(repairHeader(context, userVersion, builder, FormatVersion.SAVE_VERSION_WITH_RECORD, false)).isTrue(); + } + + private void repairHeader(final FDBRecordContext context, final int userVersion, + final FDBRecordStore.Builder builder, boolean leavePotentiallyCorruptIndexesReadable) { + assertThat(repairHeader(context, userVersion, builder, FormatVersion.SAVE_VERSION_WITH_RECORD, leavePotentiallyCorruptIndexesReadable)).isTrue(); } private boolean repairHeader(final FDBRecordContext context, final int userVersion, final FDBRecordStore.Builder builder, final FormatVersion minimumPossibleFormatVersion) { + return repairHeader(context, userVersion, builder, minimumPossibleFormatVersion, false); + } + + private boolean repairHeader(final FDBRecordContext context, final int userVersion, + final FDBRecordStore.Builder builder, final FormatVersion minimumPossibleFormatVersion, + boolean leavePotentiallyCorruptIndexesReadable) { final NonnullPair result = context.asyncToSync(FDBStoreTimer.Waits.WAIT_CHECK_VERSION, - builder.repairMissingHeader(userVersion, minimumPossibleFormatVersion)); + builder.repairMissingHeader(userVersion, minimumPossibleFormatVersion, leavePotentiallyCorruptIndexesReadable)); recordStore = result.getRight(); return result.getLeft(); }