Skip to content

Commit 266979f

Browse files
committed
Fix READONCE IOContext usage
1 parent 352b10a commit 266979f

File tree

3 files changed

+31
-8
lines changed

3 files changed

+31
-8
lines changed

server/src/main/java/org/elasticsearch/index/store/FsDirectoryFactory.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ public IndexInput openInput(String name, IOContext context) throws IOException {
123123
// we need to do these checks on the outer directory since the inner doesn't know about pending deletes
124124
ensureOpen();
125125
ensureCanRead(name);
126+
// we switch the context here since mmap checks for the READONCE context by identity
127+
context = context == Store.READONCE_CHECKSUM ? IOContext.READONCE : context;
126128
// we only use the mmap to open inputs. Everything else is managed by the NIOFSDirectory otherwise
127129
// we might run into trouble with files that are pendingDelete in one directory but still
128130
// listed in listAll() from the other. We on the other hand don't want to list files from both dirs

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@
2828
import org.apache.lucene.store.ByteArrayDataInput;
2929
import org.apache.lucene.store.ChecksumIndexInput;
3030
import org.apache.lucene.store.Directory;
31+
import org.apache.lucene.store.FilterDirectory;
3132
import org.apache.lucene.store.IOContext;
3233
import org.apache.lucene.store.IndexInput;
3334
import org.apache.lucene.store.IndexOutput;
3435
import org.apache.lucene.store.Lock;
3536
import org.apache.lucene.store.NIOFSDirectory;
37+
import org.apache.lucene.store.ReadAdvice;
3638
import org.apache.lucene.util.ArrayUtil;
3739
import org.apache.lucene.util.BytesRef;
3840
import org.apache.lucene.util.Version;
@@ -147,8 +149,15 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref
147149
* Specific {@link IOContext} indicating that we will read only the Lucene file footer (containing the file checksum)
148150
* See {@link MetadataSnapshot#checksumFromLuceneFile}.
149151
*/
150-
// TODO Lucene 10 upgrade
151-
public static final IOContext READONCE_CHECKSUM = IOContext.READONCE;
152+
public static final IOContext READONCE_CHECKSUM = createReadOnceContext();
153+
154+
// while equivalent, these different read once contexts are checked by identity in directory implementations
155+
private static IOContext createReadOnceContext() {
156+
var context = IOContext.READONCE.withReadAdvice(ReadAdvice.SEQUENTIAL);
157+
assert context != IOContext.READONCE;
158+
assert context.equals(IOContext.READONCE);
159+
return context;
160+
}
152161

153162
private final AtomicBoolean isClosed = new AtomicBoolean(false);
154163
private final StoreDirectory directory;
@@ -911,6 +920,18 @@ public static boolean isReadAsHash(String file) {
911920
return SEGMENT_INFO_EXTENSION.equals(IndexFileNames.getExtension(file)) || file.startsWith(IndexFileNames.SEGMENTS + "_");
912921
}
913922

923+
// We select the read once context carefully here since these constants, while equivalent are
924+
// checked by identity in the different directory implementations.
925+
private static IOContext contextForDirectory(String filename, Directory directory) {
926+
if (directory instanceof StoreDirectory && filename.startsWith(IndexFileNames.SEGMENTS) == false) {
927+
return READONCE_CHECKSUM;
928+
}
929+
if (FilterDirectory.unwrap(directory) instanceof FsDirectoryFactory.HybridDirectory) {
930+
return READONCE_CHECKSUM;
931+
}
932+
return IOContext.READONCE;
933+
}
934+
914935
private static void checksumFromLuceneFile(
915936
Directory directory,
916937
String file,
@@ -920,7 +941,7 @@ private static void checksumFromLuceneFile(
920941
boolean readFileAsHash,
921942
BytesRef writerUuid
922943
) throws IOException {
923-
try (IndexInput in = directory.openInput(file, READONCE_CHECKSUM)) {
944+
try (IndexInput in = directory.openInput(file, contextForDirectory(file, directory))) {
924945
final long length = in.length();
925946
if (length < CodecUtil.footerLength()) {
926947
// If the file isn't long enough to contain the footer then verifying it triggers an IAE, but really it's corrupted

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/SearchableSnapshotDirectoryTests.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -401,9 +401,9 @@ public void testChecksumBlobContainerIndexInput() throws Exception {
401401
false, // no prewarming in this test because we want to ensure that files are accessed on purpose
402402
(directory, snapshotDirectory) -> {
403403
for (String fileName : randomSubsetOf(Arrays.asList(snapshotDirectory.listAll()))) {
404-
final long checksum;
405-
try (IndexInput input = directory.openInput(fileName, Store.READONCE_CHECKSUM)) {
406-
checksum = CodecUtil.checksumEntireFile(input);
404+
final long expectedChecksum;
405+
try (IndexInput input = directory.openInput(fileName, IOContext.READONCE)) {
406+
expectedChecksum = CodecUtil.checksumEntireFile(input);
407407
}
408408

409409
final long snapshotChecksum;
@@ -418,9 +418,9 @@ public void testChecksumBlobContainerIndexInput() throws Exception {
418418
}
419419

420420
assertThat(
421-
"Expected checksum [" + checksum + "] but got [" + snapshotChecksum + ']',
421+
"Expected checksum [" + expectedChecksum + "] but got [" + snapshotChecksum + ']',
422422
snapshotChecksum,
423-
equalTo(checksum)
423+
equalTo(expectedChecksum)
424424
);
425425
assertThat(
426426
"File [" + fileName + "] should have been read from heap",

0 commit comments

Comments
 (0)