Fix Lucene error from ChecksumException to TimeoutException

fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBIndexInput.java

@@ -264,9 +264,9 @@
         }
         try {
             int probe = (int)(absolutePosition() % fileReference.getBlockSize());
-            position++;
             byte[] data = getCurrentData();
             verify(data != null, "current Data is null: " + fileName + " " + fileReference.getId());
+            position++;
             return data[probe];
         } catch (RecordCoreException ex) {
             throw LuceneExceptions.toIoException(ex, null);

fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryFailuresTest.java

@@ -24,14 +24,28 @@
 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.BooleanSource;
 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;
+import org.junit.jupiter.api.function.Executable;
+import org.junit.jupiter.params.ParameterizedTest;
 
 import javax.annotation.Nonnull;
 import java.io.IOException;
@@ -46,6 +60,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 +235,51 @@
         assertCorrectMetricCount(LuceneEvents.Counts.LUCENE_RENAME_FILE, 1);
     }
 
+    @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 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());
+        }
+    }
+
+    /**
+     * 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 +297,14 @@
                 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.
      */

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,6 +160,7 @@ public static class FailureDescription {
         @Nonnull
         private final Exception exception;
         private final long count;
+        private final long maxFailures;
 
         /**
          * Create a failure  desciption.
@@ -155,9 +170,26 @@ public static class FailureDescription {
          * @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

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<byte[]> readBlock(@Nonnull final IndexInput requestingI
                 });
     }
 
+    @Nullable
+    @Override
+    public <T> T asyncToSync(@Nonnull final StoreTimer.Wait event, @Nonnull final CompletableFuture<T> 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 {