From f53df476a1af98b32644bce540ef4652971731b4 Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 4 Nov 2026 15:55:14 -0500 Subject: [PATCH 1/3] Fix Lucene exception misalignment: CorruptIndexException due to timeout should have the cause be the original exception --- .../codec/LuceneOptimizedDocValuesFormat.java | 19 +++- ...ceneOptimizedDocValuesFormatErrorTest.java | 97 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java index ed1134c8b49..d98d94c5fcc 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java @@ -20,12 +20,14 @@ package com.apple.foundationdb.record.lucene.codec; +import com.apple.foundationdb.record.lucene.LuceneExceptions; import com.google.auto.service.AutoService; import org.apache.lucene.codecs.DocValuesConsumer; import org.apache.lucene.codecs.DocValuesFormat; import org.apache.lucene.codecs.DocValuesProducer; import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat; import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.index.SegmentReadState; @@ -71,7 +73,22 @@ private class LazyDocValuesProducer extends DocValuesProducer { public LazyDocValuesProducer(SegmentReadState state) { - docValuesProducer = LazyCloseable.supply(() -> docValuesFormat.fieldsProducer(state)); + docValuesProducer = LazyCloseable.supply(() -> { + try { + return docValuesFormat.fieldsProducer(state); + } catch (CorruptIndexException e) { + // The Lucene80DocValuesProducer constructor calls CodecUtil.checkFooter even if reading the header + // failed. So, an FDB error (e.g. a timeout) while reading the header surfaces as a + // CorruptIndexException that only has the FDB error as a suppressed exception, and callers that + // look at the cause chain would report corruption. + for (final Throwable suppressed : e.getSuppressed()) { + if (suppressed instanceof RuntimeException) { + throw LuceneExceptions.toIoException(suppressed, e); + } + } + throw e; + } + }); } @Override diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java new file mode 100644 index 00000000000..5268ca276a0 --- /dev/null +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java @@ -0,0 +1,97 @@ +/* + * LuceneOptimizedDocValuesFormatErrorTest.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2026 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.lucene.codec; + +import com.apple.foundationdb.record.RecordCoreException; +import org.apache.lucene.codecs.DocValuesConsumer; +import org.apache.lucene.codecs.DocValuesFormat; +import org.apache.lucene.codecs.DocValuesProducer; +import org.apache.lucene.index.CorruptIndexException; +import org.apache.lucene.index.SegmentReadState; +import org.apache.lucene.index.SegmentWriteState; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.Arrays; + +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests how {@link LuceneOptimizedDocValuesFormat} reports failures to open the underlying doc values producer. + */ +class LuceneOptimizedDocValuesFormatErrorTest { + + /** + * {@code Lucene80DocValuesProducer} calls {@code CodecUtil.checkFooter} even if reading the header failed, so an + * FDB error (like a timeout) during the header read comes out as a {@link CorruptIndexException} that only has + * the FDB error as a suppressed exception. Make sure the FDB error is the cause, so that callers do not + * mistake it for corruption. + */ + @Test + void suppressedRuntimeExceptionBecomesCause() { + final RecordCoreException timeout = new RecordCoreException("Timeout waiting for asynchronous operation to complete"); + final CorruptIndexException corrupt = new CorruptIndexException("checksum failed (hardware problem?)", "test"); + corrupt.addSuppressed(timeout); + + final IOException thrown = assertThrows(IOException.class, () -> invokeProducerCall(corrupt)); + + assertSame(timeout, thrown.getCause()); + assertTrue(Arrays.asList(thrown.getSuppressed()).contains(corrupt)); + } + + @Test + void genuineCorruptionHasNoCause() { + final CorruptIndexException corrupt = new CorruptIndexException("checksum failed (hardware problem?)", "test"); + + final CorruptIndexException thrown = assertThrows(CorruptIndexException.class, () -> invokeProducerCall(corrupt)); + + assertSame(corrupt, thrown); + assertNull(thrown.getCause()); + } + + private static void invokeProducerCall(final IOException failure) throws IOException { + final DocValuesProducer producer = new LuceneOptimizedDocValuesFormat(new FailingDocValuesFormat(failure)) + .fieldsProducer(null); + producer.getSorted(null); + } + + private static class FailingDocValuesFormat extends DocValuesFormat { + private final IOException failure; + + FailingDocValuesFormat(final IOException failure) { + super("Failing"); + this.failure = failure; + } + + @Override + public DocValuesConsumer fieldsConsumer(final SegmentWriteState state) { + throw new UnsupportedOperationException(); + } + + @Override + public DocValuesProducer fieldsProducer(final SegmentReadState state) throws IOException { + throw failure; + } + } +} From 5e0ea94e2b8fa6a0b831268a220003f7cb5fcc4c Mon Sep 17 00:00:00 2001 From: ohad Date: Fri, 6 Nov 2026 13:42:41 -0500 Subject: [PATCH 2/3] PR Comments: Don't change the exception structure, rather don't count the byte read unless the read actually succeeded --- .../codec/LuceneOptimizedDocValuesFormat.java | 19 +--- .../lucene/directory/FDBIndexInput.java | 2 +- ...ceneOptimizedDocValuesFormatErrorTest.java | 97 ------------------- .../directory/FDBDirectoryFailuresTest.java | 61 ++++++++++++ .../directory/InjectedFailureRepository.java | 35 ++++++- .../lucene/directory/MockedFDBDirectory.java | 13 +++ 6 files changed, 109 insertions(+), 118 deletions(-) delete mode 100644 fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java index d98d94c5fcc..ed1134c8b49 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormat.java @@ -20,14 +20,12 @@ package com.apple.foundationdb.record.lucene.codec; -import com.apple.foundationdb.record.lucene.LuceneExceptions; import com.google.auto.service.AutoService; import org.apache.lucene.codecs.DocValuesConsumer; import org.apache.lucene.codecs.DocValuesFormat; import org.apache.lucene.codecs.DocValuesProducer; import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat; import org.apache.lucene.index.BinaryDocValues; -import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.index.SegmentReadState; @@ -73,22 +71,7 @@ private class LazyDocValuesProducer extends DocValuesProducer { public LazyDocValuesProducer(SegmentReadState state) { - docValuesProducer = LazyCloseable.supply(() -> { - try { - return docValuesFormat.fieldsProducer(state); - } catch (CorruptIndexException e) { - // The Lucene80DocValuesProducer constructor calls CodecUtil.checkFooter even if reading the header - // failed. So, an FDB error (e.g. a timeout) while reading the header surfaces as a - // CorruptIndexException that only has the FDB error as a suppressed exception, and callers that - // look at the cause chain would report corruption. - for (final Throwable suppressed : e.getSuppressed()) { - if (suppressed instanceof RuntimeException) { - throw LuceneExceptions.toIoException(suppressed, e); - } - } - throw e; - } - }); + docValuesProducer = LazyCloseable.supply(() -> docValuesFormat.fieldsProducer(state)); } @Override diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java index 4c4218e1890..db1ad7ea0c9 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java @@ -268,8 +268,8 @@ public byte readByte() throws IOException { } try { int probe = (int)(absolutePosition() % fileReference.getBlockSize()); - position++; byte[] data = getCurrentData(); + position++; verify(data != null, "current Data is null: " + fileName + " " + fileReference.getId()); return data[probe]; } catch (RecordCoreException ex) { diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java deleted file mode 100644 index 5268ca276a0..00000000000 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/codec/LuceneOptimizedDocValuesFormatErrorTest.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * LuceneOptimizedDocValuesFormatErrorTest.java - * - * This source file is part of the FoundationDB open source project - * - * Copyright 2015-2026 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.lucene.codec; - -import com.apple.foundationdb.record.RecordCoreException; -import org.apache.lucene.codecs.DocValuesConsumer; -import org.apache.lucene.codecs.DocValuesFormat; -import org.apache.lucene.codecs.DocValuesProducer; -import org.apache.lucene.index.CorruptIndexException; -import org.apache.lucene.index.SegmentReadState; -import org.apache.lucene.index.SegmentWriteState; -import org.junit.jupiter.api.Test; - -import java.io.IOException; -import java.util.Arrays; - -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * Tests how {@link LuceneOptimizedDocValuesFormat} reports failures to open the underlying doc values producer. - */ -class LuceneOptimizedDocValuesFormatErrorTest { - - /** - * {@code Lucene80DocValuesProducer} calls {@code CodecUtil.checkFooter} even if reading the header failed, so an - * FDB error (like a timeout) during the header read comes out as a {@link CorruptIndexException} that only has - * the FDB error as a suppressed exception. Make sure the FDB error is the cause, so that callers do not - * mistake it for corruption. - */ - @Test - void suppressedRuntimeExceptionBecomesCause() { - final RecordCoreException timeout = new RecordCoreException("Timeout waiting for asynchronous operation to complete"); - final CorruptIndexException corrupt = new CorruptIndexException("checksum failed (hardware problem?)", "test"); - corrupt.addSuppressed(timeout); - - final IOException thrown = assertThrows(IOException.class, () -> invokeProducerCall(corrupt)); - - assertSame(timeout, thrown.getCause()); - assertTrue(Arrays.asList(thrown.getSuppressed()).contains(corrupt)); - } - - @Test - void genuineCorruptionHasNoCause() { - final CorruptIndexException corrupt = new CorruptIndexException("checksum failed (hardware problem?)", "test"); - - final CorruptIndexException thrown = assertThrows(CorruptIndexException.class, () -> invokeProducerCall(corrupt)); - - assertSame(corrupt, thrown); - assertNull(thrown.getCause()); - } - - private static void invokeProducerCall(final IOException failure) throws IOException { - final DocValuesProducer producer = new LuceneOptimizedDocValuesFormat(new FailingDocValuesFormat(failure)) - .fieldsProducer(null); - producer.getSorted(null); - } - - private static class FailingDocValuesFormat extends DocValuesFormat { - private final IOException failure; - - FailingDocValuesFormat(final IOException failure) { - super("Failing"); - this.failure = failure; - } - - @Override - public DocValuesConsumer fieldsConsumer(final SegmentWriteState state) { - throw new UnsupportedOperationException(); - } - - @Override - public DocValuesProducer fieldsProducer(final SegmentReadState state) throws IOException { - throw failure; - } - } -} diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java index 4f88f9c263a..0863f619617 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java @@ -24,12 +24,23 @@ import com.apple.foundationdb.record.lucene.LuceneConcurrency; import com.apple.foundationdb.record.lucene.LuceneEvents; import com.apple.foundationdb.record.lucene.LuceneExceptions; +import com.apple.foundationdb.record.lucene.codec.LuceneOptimizedCodec; import com.apple.foundationdb.record.provider.common.StoreTimer; import com.apple.foundationdb.record.provider.foundationdb.FDBExceptions; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; import com.apple.foundationdb.record.provider.foundationdb.FDBStoreTimer; import com.apple.foundationdb.subspace.Subspace; import com.apple.test.Tags; +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.SortedDocValuesField; +import org.apache.lucene.index.CorruptIndexException; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.util.BytesRef; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -46,6 +57,7 @@ import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_GET_INCREMENT; import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_LIST_ALL; import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_READ_BLOCK; +import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_WAIT_FOR_DATA_BLOCK; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -220,6 +232,47 @@ public void testRename() { assertCorrectMetricCount(LuceneEvents.Counts.LUCENE_RENAME_FILE, 1); } + @Test + public void testReadByteFailureDoesNotAdvancePosition() throws Exception { + writeFile("testReadByte", CodecUtil.footerLength() + 1); + addFailure(LUCENE_READ_BLOCK, + new LuceneConcurrency.AsyncToSyncTimeoutException("Blah", new TimeoutException("Blah")), 0); + + try (FDBIndexInput input = new FDBIndexInput("testReadByte", directory)) { + final IOException ex = assertThrows(IOException.class, input::readByte); + assertTrue(ex.getCause() instanceof LuceneConcurrency.AsyncToSyncTimeoutException); + // A byte that failed to read must not be counted as read + assertEquals(0, input.getFilePointer()); + } + } + + /** + * Open the doc values of a real segment while the first read of data times out. {@code Lucene80DocValuesProducer} + * calls {@code CodecUtil.checkFooter} even if reading the header failed. If the failed read advanced the position, + * the checksum (which does not include the byte that was not read) does not match the footer, and the timeout is + * hidden behind a {@link CorruptIndexException}. + */ + @Test + public void testDocValuesTimeoutThrowsTimeoutException() throws Exception { + try (IndexWriter writer = new IndexWriter(directory, + new IndexWriterConfig(new StandardAnalyzer()).setCodec(LuceneOptimizedCodec.CODEC))) { + final Document document = new Document(); + document.add(new SortedDocValuesField("dv", new BytesRef("value"))); + writer.addDocument(document); + writer.commit(); + } + + try (DirectoryReader reader = DirectoryReader.open(directory)) { + final LeafReader leafReader = reader.leaves().get(0).reader(); + // Fail the async to sync with a timeout only once + injectedFailures.addTransientFailure(LUCENE_WAIT_FOR_DATA_BLOCK, + new LuceneConcurrency.AsyncToSyncTimeoutException("Blah", new TimeoutException("Blah")), 0, 1); + final IOException ex = assertThrows(IOException.class, () -> leafReader.getSortedDocValues("dv")); + assertTrue(ex.getCause() instanceof LuceneConcurrency.AsyncToSyncTimeoutException, + () -> "Expected the timeout to be the cause, but got: " + ex); + } + } + private void assertCorrectMetricCount(StoreTimer.Event metric, int expectedValue) { assertEquals(expectedValue, timer.getCount(metric), () -> "Incorrect call count for metric " + metric); @@ -237,6 +290,14 @@ private void assertMetricCountAtMost(StoreTimer.Event metric, int maximumValue) timer.getCount(metric), lessThanOrEqualTo(maximumValue)); } + private void writeFile(final String name, final int length) throws IOException { + final byte[] bytes = new byte[length]; + random.nextBytes(bytes); + try (FDBIndexOutput output = new FDBIndexOutput(name, directory)) { + output.writeBytes(bytes, bytes.length); + } + } + /* * Override default behavior to create a mocked directory. */ diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java index 9de27677ef5..f916b73a873 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java @@ -41,6 +41,7 @@ public enum Methods { LUCENE_GET_INCREMENT, LUCENE_GET_FDB_LUCENE_FILE_REFERENCE_ASYNC, LUCENE_READ_BLOCK, + LUCENE_WAIT_FOR_DATA_BLOCK, /** A failure in the asyncToSync call for reading a data block (future can continue). */ LUCENE_LIST_ALL, LUCENE_GET_FILE_REFERENCE_CACHE_ASYNC, LUCENE_DELETE_FILE_INTERNAL, @@ -72,6 +73,18 @@ public void addFailure(@Nonnull Methods method, @Nonnull Exception exception, lo failureDescriptions.put(method, new FailureDescription(method, exception, count)); } + /** + * Add a failure that happens a limited number of times. + * + * @param method the method that will fail once + * @param exception the exception to throw + * @param count the number of "clean" executions to allow before throwing the exception + * @param maxFailures the number of times to fail the call before returning to normal + */ + public void addTransientFailure(@Nonnull Methods method, @Nonnull Exception exception, long count, int maxFailures) { + failureDescriptions.put(method, new FailureDescription(method, exception, count, maxFailures)); + } + public void removeFailure(@Nonnull Methods method) { failureDescriptions.remove(method); invocationCounts.remove(method); @@ -131,7 +144,8 @@ private void checkFailure(@Nonnull final Methods method) throws Exception { if (failureDescription != null) { AtomicLong count = invocationCounts.computeIfAbsent(method, m -> new AtomicLong(0)); long invocations = count.incrementAndGet(); - if (invocations > failureDescription.getCount()) { + if (invocations > failureDescription.getCount() + && invocations - failureDescription.getCount() <= failureDescription.getMaxFailures()) { throw failureDescription.getException(); } } @@ -146,17 +160,34 @@ public static class FailureDescription { @Nonnull private final Exception exception; private final long count; + private final long maxFailures; /** - * Create a failure desciption. + * Create a failure description. * @param method the method that will fail: {@link Methods} should contain the entire list * @param exception the exception to throw * @param count the number of "clean" executions to allow before starting to throw the exception */ public FailureDescription(@Nonnull final Methods method, @Nonnull final Exception exception, final long count) { + this(method, exception, count, Long.MAX_VALUE); + } + + /** + * Create a failure description. + * @param method the method that will fail: {@link Methods} should contain the entire list + * @param exception the exception to throw + * @param count the number of "clean" executions to allow before starting to throw the exception + * @param maxFailures the number of executions that throw the exception, before executions are clean again + */ + public FailureDescription(@Nonnull final Methods method, @Nonnull final Exception exception, final long count, final long maxFailures) { this.method = method; this.exception = exception; this.count = count; + this.maxFailures = maxFailures; + } + + public long getMaxFailures() { + return maxFailures; } @Nonnull diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectory.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectory.java index 9b79c435991..efcabffb95f 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectory.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectory.java @@ -20,7 +20,9 @@ package com.apple.foundationdb.record.lucene.directory; +import com.apple.foundationdb.record.lucene.LuceneEvents; import com.apple.foundationdb.record.lucene.LucenePrimaryKeySegmentIndex; +import com.apple.foundationdb.record.provider.common.StoreTimer; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; import com.apple.foundationdb.record.util.pair.NonnullPair; import com.apple.foundationdb.subspace.Subspace; @@ -43,6 +45,7 @@ import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_GET_PRIMARY_KEY_SEGMENT_INDEX; import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_LIST_ALL; import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_READ_BLOCK; +import static com.apple.foundationdb.record.lucene.directory.InjectedFailureRepository.Methods.LUCENE_WAIT_FOR_DATA_BLOCK; /** * An {@link FDBDirectory} used for testing. It has the ability to delegate calls to the production implementation as well as fail as needed. @@ -86,6 +89,16 @@ public CompletableFuture readBlock(@Nonnull final IndexInput requestingI }); } + @Nullable + @Override + public T asyncToSync(@Nonnull final StoreTimer.Wait event, @Nonnull final CompletableFuture async) { + // Unlike a failure in readBlock, this does not fail the future. Like a real timeout, the same data can be read later + if (event == LuceneEvents.Waits.WAIT_LUCENE_GET_DATA_BLOCK) { + injectedFailures.checkFailureForCoreException(LUCENE_WAIT_FOR_DATA_BLOCK); + } + return super.asyncToSync(event, async); + } + @Nonnull @Override public String[] listAll() throws IOException { From eeea980c894a086d5cfe7a8e78437fb7989eb5ac Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 10 Nov 2026 14:21:54 -0500 Subject: [PATCH 3/3] PR comments: Move increment, add test, fix style --- .../record/lucene/directory/FDBIndexInput.java | 2 +- .../lucene/directory/FDBDirectoryFailuresTest.java | 13 ++++++++++--- .../lucene/directory/InjectedFailureRepository.java | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java index db1ad7ea0c9..1317dd56b39 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java @@ -269,8 +269,8 @@ public byte readByte() throws IOException { try { int probe = (int)(absolutePosition() % fileReference.getBlockSize()); byte[] data = getCurrentData(); - position++; verify(data != null, "current Data is null: " + fileName + " " + fileReference.getId()); + position++; return data[probe]; } catch (RecordCoreException ex) { throw LuceneExceptions.toIoException(ex, null); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java index 0863f619617..e4e0f21f175 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java @@ -30,6 +30,7 @@ import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; import com.apple.foundationdb.record.provider.foundationdb.FDBStoreTimer; import com.apple.foundationdb.subspace.Subspace; +import com.apple.test.BooleanSource; import com.apple.test.Tags; import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.codecs.CodecUtil; @@ -43,6 +44,8 @@ import org.apache.lucene.util.BytesRef; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; import javax.annotation.Nonnull; import java.io.IOException; @@ -232,14 +235,18 @@ public void testRename() { assertCorrectMetricCount(LuceneEvents.Counts.LUCENE_RENAME_FILE, 1); } - @Test - public void testReadByteFailureDoesNotAdvancePosition() throws Exception { + @ParameterizedTest + @BooleanSource("multipleBytes") + public void testReadByteFailureDoesNotAdvancePosition(boolean multipleBytes) throws Exception { writeFile("testReadByte", CodecUtil.footerLength() + 1); addFailure(LUCENE_READ_BLOCK, new LuceneConcurrency.AsyncToSyncTimeoutException("Blah", new TimeoutException("Blah")), 0); try (FDBIndexInput input = new FDBIndexInput("testReadByte", directory)) { - final IOException ex = assertThrows(IOException.class, input::readByte); + final Executable readBytes = multipleBytes ? + () -> input.readBytes(new byte[1], 0, 1) : + input::readByte; + final IOException ex = assertThrows(IOException.class, readBytes); assertTrue(ex.getCause() instanceof LuceneConcurrency.AsyncToSyncTimeoutException); // A byte that failed to read must not be counted as read assertEquals(0, input.getFilePointer()); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java index f916b73a873..4d463ff9799 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/InjectedFailureRepository.java @@ -174,6 +174,7 @@ public FailureDescription(@Nonnull final Methods method, @Nonnull final Exceptio /** * Create a failure description. + * * @param method the method that will fail: {@link Methods} should contain the entire list * @param exception the exception to throw * @param count the number of "clean" executions to allow before starting to throw the exception