From 225b0a7c683c3ba1cab36c20d6029cd3c60a83a7 Mon Sep 17 00:00:00 2001 From: Asim Mahmood Date: Mon, 24 Mar 2025 11:04:18 -0700 Subject: [PATCH 1/3] Switch from IOContext.DEFAULT(RANDOM) to READONLY for sequential cases * Lucene 10 changed the IOContext.DEFAULT from sequential to random, which makes sense for search use case: https://github.com/apache/lucene/pull/13244 * place we read a file only once, its better to switch to READONLY(sequential) * this should only be in cases the file is read by the same thread that opened it, e.g. it won't work for RemoteStore that does async upload Signed-off-by: Asim Mahmood --- .../org/opensearch/common/settings/KeyStoreWrapper.java | 2 +- .../java/org/opensearch/gateway/MetadataStateFormat.java | 6 +++--- .../opensearch/index/shard/RemoteStoreRefreshListener.java | 2 +- .../main/java/org/opensearch/index/translog/Checkpoint.java | 2 +- .../indices/replication/SegmentFileTransferHandler.java | 2 +- .../repositories/blobstore/BlobStoreRepository.java | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java index 81fb1309df310..d5ec37516a678 100644 --- a/server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java @@ -527,7 +527,7 @@ public synchronized void save(Path configDir, char[] password) throws Exception NIOFSDirectory directory = new NIOFSDirectory(configDir); // write to tmp file first, then overwrite String tmpFile = KEYSTORE_FILENAME + ".tmp"; - try (IndexOutput output = EndiannessReverserUtil.createOutput(directory, tmpFile, IOContext.DEFAULT)) { + try (IndexOutput output = EndiannessReverserUtil.createOutput(directory, tmpFile, IOContext.READONCE)) { CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); output.writeByte(password.length == 0 ? (byte) 0 : (byte) 1); diff --git a/server/src/main/java/org/opensearch/gateway/MetadataStateFormat.java b/server/src/main/java/org/opensearch/gateway/MetadataStateFormat.java index ad47ca66129bb..cb55c35a9b7e6 100644 --- a/server/src/main/java/org/opensearch/gateway/MetadataStateFormat.java +++ b/server/src/main/java/org/opensearch/gateway/MetadataStateFormat.java @@ -120,7 +120,7 @@ private void writeStateToFirstLocation(final T state, Path stateLocation, Direct throws WriteStateException { try { deleteFileIfExists(stateLocation, stateDir, tmpFileName); - try (IndexOutput out = EndiannessReverserUtil.createOutput(stateDir, tmpFileName, IOContext.DEFAULT)) { + try (IndexOutput out = EndiannessReverserUtil.createOutput(stateDir, tmpFileName, IOContext.READONCE)) { CodecUtil.writeHeader(out, STATE_FILE_CODEC, STATE_FILE_VERSION); out.writeInt(FORMAT.index()); try (XContentBuilder builder = newXContentBuilder(FORMAT, new IndexOutputOutputStream(out) { @@ -155,7 +155,7 @@ private static void copyStateToExtraLocations(List> state Directory extraStateDir = extraStatePathAndDir.v2(); try { deleteFileIfExists(extraStateLocation, extraStateDir, tmpFileName); - extraStateDir.copyFrom(srcStateDir, tmpFileName, tmpFileName, IOContext.DEFAULT); + extraStateDir.copyFrom(srcStateDir, tmpFileName, tmpFileName, IOContext.READONCE); extraStateDir.sync(Collections.singleton(tmpFileName)); } catch (Exception e) { throw new WriteStateException(false, "failed to copy tmp state file to extra location " + extraStateLocation, e); @@ -309,7 +309,7 @@ protected XContentBuilder newXContentBuilder(XContentType type, OutputStream str */ public final T read(NamedXContentRegistry namedXContentRegistry, Path file) throws IOException { try (Directory dir = newDirectory(file.getParent())) { - try (IndexInput indexInput = EndiannessReverserUtil.openInput(dir, file.getFileName().toString(), IOContext.DEFAULT)) { + try (IndexInput indexInput = EndiannessReverserUtil.openInput(dir, file.getFileName().toString(), IOContext.READONCE)) { // We checksum the entire file before we even go and parse it. If it's corrupted we barf right here. CodecUtil.checksumEntireFile(indexInput); CodecUtil.checkHeader(indexInput, STATE_FILE_CODEC, MIN_COMPATIBLE_STATE_FILE_VERSION, STATE_FILE_VERSION); diff --git a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java index 8ace4848806d7..702928771f131 100644 --- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java +++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java @@ -459,7 +459,7 @@ private void uploadNewSegments( batchUploadListener.onFailure(ex); }); statsListener.beforeUpload(src); - remoteDirectory.copyFrom(storeDirectory, src, IOContext.DEFAULT, aggregatedListener, isLowPriorityUpload()); + remoteDirectory.copyFrom(storeDirectory, src, IOContext.READONCE, aggregatedListener, isLowPriorityUpload()); } } diff --git a/server/src/main/java/org/opensearch/index/translog/Checkpoint.java b/server/src/main/java/org/opensearch/index/translog/Checkpoint.java index d309564ef5d32..8d63fab3c8c31 100644 --- a/server/src/main/java/org/opensearch/index/translog/Checkpoint.java +++ b/server/src/main/java/org/opensearch/index/translog/Checkpoint.java @@ -201,7 +201,7 @@ public String toString() { public static Checkpoint read(Path path) throws IOException { try (Directory dir = new NIOFSDirectory(path.getParent())) { - try (IndexInput indexInput = dir.openInput(path.getFileName().toString(), IOContext.DEFAULT)) { + try (IndexInput indexInput = dir.openInput(path.getFileName().toString(), IOContext.READONCE)) { // We checksum the entire file before we even go and parse it. If it's corrupted we barf right here. CodecUtil.checksumEntireFile(indexInput); final int fileVersion = CodecUtil.checkHeader(indexInput, CHECKPOINT_CODEC, VERSION_LUCENE_BIG_ENDIAN, CURRENT_VERSION); diff --git a/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java b/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java index 756751cf0b87d..24b2b6affef8f 100644 --- a/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java +++ b/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java @@ -110,7 +110,7 @@ protected void onNewResource(StoreFileMetadata md) throws IOException { // Segments* files require IOContext.READONCE // https://github.com/apache/lucene/blob/b2d3a2b37e00f19a74949097736be8fd64745f61/lucene/test-framework/src/java/org/apache/lucene/tests/store/MockDirectoryWrapper.java#L817 if (md.name().startsWith(IndexFileNames.SEGMENTS) == false) { - final IndexInput indexInput = store.directory().openInput(md.name(), IOContext.DEFAULT); + final IndexInput indexInput = store.directory().openInput(md.name(), IOContext.READONCE); currentInput = new InputStreamIndexInput(indexInput, md.length()) { @Override public void close() throws IOException { diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java index 54c4a3332e005..c10991def43a3 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java @@ -4650,7 +4650,7 @@ private void snapshotFile( ) throws IOException { final BlobContainer shardContainer = shardContainer(indexId, shardId); final String file = fileInfo.physicalName(); - try (IndexInput indexInput = store.openVerifyingInput(file, IOContext.DEFAULT, fileInfo.metadata())) { + try (IndexInput indexInput = store.openVerifyingInput(file, IOContext.READONCE, fileInfo.metadata())) { for (int i = 0; i < fileInfo.numberOfParts(); i++) { final long partBytes = fileInfo.partBytes(i); From d1b2c8ec2d812fea0da88e8ded9d811434160b77 Mon Sep 17 00:00:00 2001 From: Asim Mahmood Date: Tue, 25 Mar 2025 18:18:01 +0000 Subject: [PATCH 2/3] SegmentFileTranser should not have been changed Signed-off-by: Asim Mahmood --- .../indices/replication/SegmentFileTransferHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java b/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java index 24b2b6affef8f..756751cf0b87d 100644 --- a/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java +++ b/server/src/main/java/org/opensearch/indices/replication/SegmentFileTransferHandler.java @@ -110,7 +110,7 @@ protected void onNewResource(StoreFileMetadata md) throws IOException { // Segments* files require IOContext.READONCE // https://github.com/apache/lucene/blob/b2d3a2b37e00f19a74949097736be8fd64745f61/lucene/test-framework/src/java/org/apache/lucene/tests/store/MockDirectoryWrapper.java#L817 if (md.name().startsWith(IndexFileNames.SEGMENTS) == false) { - final IndexInput indexInput = store.directory().openInput(md.name(), IOContext.READONCE); + final IndexInput indexInput = store.directory().openInput(md.name(), IOContext.DEFAULT); currentInput = new InputStreamIndexInput(indexInput, md.length()) { @Override public void close() throws IOException { From adc0fdde5a0d85924b9e7547f7049ae0abc19e65 Mon Sep 17 00:00:00 2001 From: Asim Mahmood Date: Thu, 27 Mar 2025 18:07:30 +0000 Subject: [PATCH 3/3] Remote store should not have been changed Signed-off-by: Asim Mahmood --- .../org/opensearch/index/shard/RemoteStoreRefreshListener.java | 2 +- .../opensearch/repositories/blobstore/BlobStoreRepository.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java index 702928771f131..8ace4848806d7 100644 --- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java +++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java @@ -459,7 +459,7 @@ private void uploadNewSegments( batchUploadListener.onFailure(ex); }); statsListener.beforeUpload(src); - remoteDirectory.copyFrom(storeDirectory, src, IOContext.READONCE, aggregatedListener, isLowPriorityUpload()); + remoteDirectory.copyFrom(storeDirectory, src, IOContext.DEFAULT, aggregatedListener, isLowPriorityUpload()); } } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java index c10991def43a3..54c4a3332e005 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java @@ -4650,7 +4650,7 @@ private void snapshotFile( ) throws IOException { final BlobContainer shardContainer = shardContainer(indexId, shardId); final String file = fileInfo.physicalName(); - try (IndexInput indexInput = store.openVerifyingInput(file, IOContext.READONCE, fileInfo.metadata())) { + try (IndexInput indexInput = store.openVerifyingInput(file, IOContext.DEFAULT, fileInfo.metadata())) { for (int i = 0; i < fileInfo.numberOfParts(); i++) { final long partBytes = fileInfo.partBytes(i);