From 1d9564b0a1ab8ffd006a23e5065b6611f10fa042 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Mon, 8 Apr 2024 14:24:31 +0530 Subject: [PATCH 1/9] Add support to separate segment infos snapshot from metadata Signed-off-by: Sachin Kale --- .../opensearch/index/shard/IndexShard.java | 19 +++- .../shard/RemoteStoreRefreshListener.java | 21 ++++- .../store/RemoteSegmentStoreDirectory.java | 94 +++++++++++++------ .../indices/RemoteStoreSettings.java | 22 +++++ .../index/remote/RemoteStoreUtilsTests.java | 12 ++- .../RemoteSegmentStoreDirectoryTests.java | 61 +++--------- 6 files changed, 145 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 26dbbbcdee7c0..6496bc09a5462 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5048,15 +5048,24 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn copySegmentFiles(storeDirectory, remoteDirectory, null, uploadedSegments, overrideLocal, onFileSync); if (remoteSegmentMetadata != null) { - final SegmentInfos infosSnapshot = store.buildSegmentInfos( - remoteSegmentMetadata.getSegmentInfosBytes(), - remoteSegmentMetadata.getGeneration() - ); + final SegmentInfos infosSnapshot; + if (remoteSegmentMetadata.getSegmentInfosBytes().length == 0) { + List segmentInfosSnapshotFilenames = Arrays.stream(store.directory().listAll()) + .filter(file -> file.startsWith("segment_infos_snapshot")) + .collect(Collectors.toList()); + assert segmentInfosSnapshotFilenames.size() == 1; + infosSnapshot = SegmentInfos.readCommit(store.directory(), segmentInfosSnapshotFilenames.get(0)); + } else { + infosSnapshot = store.buildSegmentInfos( + remoteSegmentMetadata.getSegmentInfosBytes(), + remoteSegmentMetadata.getGeneration() + ); + } long processedLocalCheckpoint = Long.parseLong(infosSnapshot.getUserData().get(LOCAL_CHECKPOINT_KEY)); // delete any other commits, we want to start the engine only from a new commit made with the downloaded infos bytes. // Extra segments will be wiped on engine open. for (String file : List.of(store.directory().listAll())) { - if (file.startsWith(IndexFileNames.SEGMENTS)) { + if (file.startsWith(IndexFileNames.SEGMENTS) || file.startsWith("segment_infos_snapshot")) { store.deleteQuiet(file); } } 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 351aec6e3af6c..eb69d1b0b22da 100644 --- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java +++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java @@ -401,11 +401,30 @@ void uploadMetadata(Collection localSegmentsPostRefresh, SegmentInfos se storeDirectory, translogFileGeneration, replicationCheckpoint, - indexShard.getNodeId() + indexShard.getNodeId(), + indexShard.getRemoteStoreSettings().getClusterRemoteSegmentSeparateMetadataSegmentInfos() ); } } + String uploadSegmentInfosSnapshot(String latestSegmentsNFilename, SegmentInfos segmentInfosSnapshot) throws IOException { + final long maxSeqNoFromSegmentInfos = indexShard.getEngine().getMaxSeqNoFromSegmentInfos(segmentInfosSnapshot); + + Map userData = segmentInfosSnapshot.getUserData(); + userData.put(LOCAL_CHECKPOINT_KEY, String.valueOf(maxSeqNoFromSegmentInfos)); + userData.put(SequenceNumbers.MAX_SEQ_NO, Long.toString(maxSeqNoFromSegmentInfos)); + segmentInfosSnapshot.setUserData(userData, false); + + long commitGeneration = SegmentInfos.generationFromSegmentsFileName(latestSegmentsNFilename); + String segmentInfoSnapshotFilename = SEGMENT_INFO_SNAPSHOT_FILENAME_PREFIX + "__" + commitGeneration; + try (IndexOutput indexOutput = storeDirectory.createOutput(segmentInfoSnapshotFilename, IOContext.DEFAULT)) { + segmentInfosSnapshot.write(indexOutput); + } + storeDirectory.sync(Collections.singleton(segmentInfoSnapshotFilename)); + remoteDirectory.copyFrom(storeDirectory, segmentInfoSnapshotFilename, segmentInfoSnapshotFilename, IOContext.DEFAULT, true); + return segmentInfoSnapshotFilename; + } + private void uploadNewSegments( Collection localSegmentsPostRefresh, Map localSegmentsSizeMap, diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index ec1163fe91b6c..37ce03a892404 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -136,6 +136,30 @@ public RemoteSegmentStoreDirectory( init(); } + // Visible for testing + public static String getMetadataFilename( + String separator, + String prefix, + long primaryTerm, + long generation, + long translogGeneration, + long uploadCounter, + int metadataVersion, + String nodeId + ) { + return String.join( + separator, + prefix, + RemoteStoreUtils.invertLong(primaryTerm), + RemoteStoreUtils.invertLong(generation), + RemoteStoreUtils.invertLong(translogGeneration), + RemoteStoreUtils.invertLong(uploadCounter), + String.valueOf(Objects.hash(nodeId)), + RemoteStoreUtils.invertLong(System.currentTimeMillis()), + String.valueOf(metadataVersion) + ); + } + /** * Initializes the cache which keeps track of all the segment files uploaded to the remote segment store. * As this cache is specific to an instance of RemoteSegmentStoreDirectory, it is possible that cache becomes stale @@ -317,28 +341,6 @@ static String getMetadataFilePrefixForCommit(long primaryTerm, long generation) ); } - // Visible for testing - public static String getMetadataFilename( - long primaryTerm, - long generation, - long translogGeneration, - long uploadCounter, - int metadataVersion, - String nodeId - ) { - return String.join( - SEPARATOR, - METADATA_PREFIX, - RemoteStoreUtils.invertLong(primaryTerm), - RemoteStoreUtils.invertLong(generation), - RemoteStoreUtils.invertLong(translogGeneration), - RemoteStoreUtils.invertLong(uploadCounter), - String.valueOf(Objects.hash(nodeId)), - RemoteStoreUtils.invertLong(System.currentTimeMillis()), - String.valueOf(metadataVersion) - ); - } - // Visible for testing static long getPrimaryTerm(String[] filenameTokens) { return RemoteStoreUtils.invertLong(filenameTokens[1]); @@ -595,10 +597,13 @@ public void uploadMetadata( Directory storeDirectory, long translogGeneration, ReplicationCheckpoint replicationCheckpoint, - String nodeId + String nodeId, + boolean separateSegmentInfos ) throws IOException { synchronized (this) { - String metadataFilename = MetadataFilenameUtils.getMetadataFilename( + String metadataFilename = getMetadataFilename( + MetadataFilenameUtils.SEPARATOR, + MetadataFilenameUtils.METADATA_PREFIX, replicationCheckpoint.getPrimaryTerm(), segmentInfosSnapshot.getGeneration(), translogGeneration, @@ -621,9 +626,44 @@ public void uploadMetadata( } ByteBuffersDataOutput byteBuffersIndexOutput = new ByteBuffersDataOutput(); - segmentInfosSnapshot.write( - new ByteBuffersIndexOutput(byteBuffersIndexOutput, "Snapshot of SegmentInfos", "SegmentInfos") - ); + if (separateSegmentInfos == false) { + segmentInfosSnapshot.write( + new ByteBuffersIndexOutput(byteBuffersIndexOutput, "Snapshot of SegmentInfos", "SegmentInfos") + ); + } else { + String segmentInfoSnapshotFilename = getMetadataFilename( + MetadataFilenameUtils.SEPARATOR, + "segment_infos_snapshot", + replicationCheckpoint.getPrimaryTerm(), + segmentInfosSnapshot.getGeneration(), + translogGeneration, + metadataUploadCounter.incrementAndGet(), + RemoteSegmentMetadata.CURRENT_VERSION, + nodeId + ); + try { + try ( + IndexOutput segmentInfosIndexOutput = storeDirectory.createOutput( + segmentInfoSnapshotFilename, + IOContext.DEFAULT + ) + ) { + segmentInfosSnapshot.write(segmentInfosIndexOutput); + } + copyFrom(storeDirectory, segmentInfoSnapshotFilename, segmentInfoSnapshotFilename, IOContext.DEFAULT); + String segmentInfosSnapshotChecksum = getChecksumOfLocalFile(storeDirectory, segmentInfoSnapshotFilename); + UploadedSegmentMetadata segmentInfosSnapshotMetadata = new UploadedSegmentMetadata( + segmentInfoSnapshotFilename, + segmentInfoSnapshotFilename, + segmentInfosSnapshotChecksum, + storeDirectory.fileLength(segmentInfosSnapshotChecksum) + ); + segmentInfosSnapshotMetadata.setWrittenByMajor(segmentInfosSnapshot.getCommitLuceneVersion().major); + uploadedSegments.put(segmentInfoSnapshotFilename, segmentInfosSnapshotMetadata.toString()); + } finally { + tryAndDeleteLocalFile(segmentInfoSnapshotFilename, storeDirectory); + } + } byte[] segmentInfoSnapshotByteArray = byteBuffersIndexOutput.toArrayCopy(); metadataStreamWrapper.writeStream( diff --git a/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java b/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java index 7f2121093f8e8..ca88108defa76 100644 --- a/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java +++ b/server/src/main/java/org/opensearch/indices/RemoteStoreSettings.java @@ -65,9 +65,17 @@ public class RemoteStoreSettings { Property.Dynamic ); + public static final Setting CLUSTER_REMOTE_SEGMENT_SEPARATE_METADATA_SEGMENTINFOS_SETTING = Setting.boolSetting( + "cluster.remote_store.segemnt.separate_metadata_segmentinfos", + false, + Property.NodeScope, + Property.Dynamic + ); + private volatile TimeValue clusterRemoteTranslogBufferInterval; private volatile int minRemoteSegmentMetadataFiles; private volatile TimeValue clusterRemoteTranslogTransferTimeout; + private volatile Boolean clusterRemoteSegmentSeparateMetadataSegmentInfos; public RemoteStoreSettings(Settings settings, ClusterSettings clusterSettings) { this.clusterRemoteTranslogBufferInterval = CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings); @@ -87,6 +95,12 @@ public RemoteStoreSettings(Settings settings, ClusterSettings clusterSettings) { CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING, this::setClusterRemoteTranslogTransferTimeout ); + + this.clusterRemoteSegmentSeparateMetadataSegmentInfos = CLUSTER_REMOTE_SEGMENT_SEPARATE_METADATA_SEGMENTINFOS_SETTING.get(settings); + clusterSettings.addSettingsUpdateConsumer( + CLUSTER_REMOTE_SEGMENT_SEPARATE_METADATA_SEGMENTINFOS_SETTING, + this::setClusterRemoteSegmentSeparateMetadataSegmentInfos + ); } public TimeValue getClusterRemoteTranslogBufferInterval() { @@ -112,4 +126,12 @@ public TimeValue getClusterRemoteTranslogTransferTimeout() { private void setClusterRemoteTranslogTransferTimeout(TimeValue clusterRemoteTranslogTransferTimeout) { this.clusterRemoteTranslogTransferTimeout = clusterRemoteTranslogTransferTimeout; } + + public Boolean getClusterRemoteSegmentSeparateMetadataSegmentInfos() { + return clusterRemoteSegmentSeparateMetadataSegmentInfos; + } + + public void setClusterRemoteSegmentSeparateMetadataSegmentInfos(Boolean clusterRemoteSegmentSeparateMetadataSegmentInfos) { + this.clusterRemoteSegmentSeparateMetadataSegmentInfos = clusterRemoteSegmentSeparateMetadataSegmentInfos; + } } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java index 4d3e633848975..e1ff45840848f 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -44,7 +44,9 @@ public class RemoteStoreUtilsTests extends OpenSearchTestCase { BASE64_CHARSET_IDX_MAP = Collections.unmodifiableMap(charToIndexMap); } - private final String metadataFilename = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( + private final String metadataFilename = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, 12, 23, 34, @@ -53,7 +55,9 @@ public class RemoteStoreUtilsTests extends OpenSearchTestCase { "node-1" ); - private final String metadataFilenameDup = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( + private final String metadataFilenameDup = RemoteSegmentStoreDirectory.getMetadataFilename( + SEPARATOR, + METADATA_PREFIX, 12, 23, 34, @@ -61,7 +65,9 @@ public class RemoteStoreUtilsTests extends OpenSearchTestCase { 1, "node-2" ); - private final String metadataFilename2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( + private final String metadataFilename2 = RemoteSegmentStoreDirectory.getMetadataFilename( + SEPARATOR, + METADATA_PREFIX, 12, 13, 34, diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index b1e2028d761f0..43107ddb9bf42 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -99,47 +99,12 @@ public class RemoteSegmentStoreDirectoryTests extends IndexShardTestCase { private SegmentInfos segmentInfos; private ThreadPool threadPool; - private final String metadataFilename = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( - 12, - 23, - 34, - 1, - 1, - "node-1" - ); - - private final String metadataFilenameDup = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( - 12, - 23, - 34, - 2, - 1, - "node-2" - ); - private final String metadataFilename2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( - 12, - 13, - 34, - 1, - 1, - "node-1" - ); - private final String metadataFilename3 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( - 10, - 38, - 34, - 1, - 1, - "node-1" - ); - private final String metadataFilename4 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( - 10, - 36, - 34, - 1, - 1, - "node-1" - ); + private final String metadataFilename = RemoteSegmentStoreDirectory.getMetadataFilename(12, 23, 34, 1, 1, "node-1"); + + private final String metadataFilenameDup = RemoteSegmentStoreDirectory.getMetadataFilename(12, 23, 34, 2, 1, "node-2"); + private final String metadataFilename2 = RemoteSegmentStoreDirectory.getMetadataFilename(12, 13, 34, 1, 1, "node-1"); + private final String metadataFilename3 = RemoteSegmentStoreDirectory.getMetadataFilename(10, 38, 34, 1, 1, "node-1"); + private final String metadataFilename4 = RemoteSegmentStoreDirectory.getMetadataFilename(10, 36, 34, 1, 1, "node-1"); @Before public void setup() throws IOException { @@ -508,7 +473,7 @@ public void testIsAcquiredException() throws IOException { private List getDummyMetadataFiles(int count) { List sortedMetadataFiles = new ArrayList<>(); for (int counter = 0; counter < count; counter++) { - sortedMetadataFiles.add(RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(counter, 23, 34, 1, 1, "node-1")); + sortedMetadataFiles.add(RemoteSegmentStoreDirectory.getMetadataFilename(counter, 23, 34, 1, 1, "node-1")); } return sortedMetadataFiles; } @@ -1273,12 +1238,12 @@ private void indexDocs(int startDocId, int numberOfDocs) throws IOException { } public void testMetadataFileNameOrder() { - String file1 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 21, 23, 1, 1, ""); - String file2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 38, 38, 1, 1, ""); - String file3 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(18, 12, 26, 1, 1, ""); - String file4 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 38, 32, 10, 1, ""); - String file5 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 38, 32, 1, 1, ""); - String file6 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 38, 32, 5, 1, ""); + String file1 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 21, 23, 1, 1, ""); + String file2 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 38, 1, 1, ""); + String file3 = RemoteSegmentStoreDirectory.getMetadataFilename(18, 12, 26, 1, 1, ""); + String file4 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 10, 1, ""); + String file5 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 1, 1, ""); + String file6 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 5, 1, ""); List actualList = new ArrayList<>(List.of(file1, file2, file3, file4, file5, file6)); actualList.sort(String::compareTo); From c0b7f0667b92df9d75ebef8d342eb9d2dff3fc68 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Mon, 8 Apr 2024 14:59:19 +0530 Subject: [PATCH 2/9] Fix spotless and test compile error Signed-off-by: Sachin Kale --- .../shard/RemoteStoreRefreshListener.java | 18 --- .../index/remote/RemoteStoreUtilsTests.java | 4 +- .../RemoteSegmentStoreDirectoryTests.java | 145 ++++++++++++++++-- 3 files changed, 131 insertions(+), 36 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 eb69d1b0b22da..b133294d0f23b 100644 --- a/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java +++ b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java @@ -407,24 +407,6 @@ void uploadMetadata(Collection localSegmentsPostRefresh, SegmentInfos se } } - String uploadSegmentInfosSnapshot(String latestSegmentsNFilename, SegmentInfos segmentInfosSnapshot) throws IOException { - final long maxSeqNoFromSegmentInfos = indexShard.getEngine().getMaxSeqNoFromSegmentInfos(segmentInfosSnapshot); - - Map userData = segmentInfosSnapshot.getUserData(); - userData.put(LOCAL_CHECKPOINT_KEY, String.valueOf(maxSeqNoFromSegmentInfos)); - userData.put(SequenceNumbers.MAX_SEQ_NO, Long.toString(maxSeqNoFromSegmentInfos)); - segmentInfosSnapshot.setUserData(userData, false); - - long commitGeneration = SegmentInfos.generationFromSegmentsFileName(latestSegmentsNFilename); - String segmentInfoSnapshotFilename = SEGMENT_INFO_SNAPSHOT_FILENAME_PREFIX + "__" + commitGeneration; - try (IndexOutput indexOutput = storeDirectory.createOutput(segmentInfoSnapshotFilename, IOContext.DEFAULT)) { - segmentInfosSnapshot.write(indexOutput); - } - storeDirectory.sync(Collections.singleton(segmentInfoSnapshotFilename)); - remoteDirectory.copyFrom(storeDirectory, segmentInfoSnapshotFilename, segmentInfoSnapshotFilename, IOContext.DEFAULT, true); - return segmentInfoSnapshotFilename; - } - private void uploadNewSegments( Collection localSegmentsPostRefresh, Map localSegmentsSizeMap, diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java index e1ff45840848f..58358ec5a93fd 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -66,8 +66,8 @@ public class RemoteStoreUtilsTests extends OpenSearchTestCase { "node-2" ); private final String metadataFilename2 = RemoteSegmentStoreDirectory.getMetadataFilename( - SEPARATOR, - METADATA_PREFIX, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, 12, 13, 34, diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index 43107ddb9bf42..503baa3f08870 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -99,12 +99,57 @@ public class RemoteSegmentStoreDirectoryTests extends IndexShardTestCase { private SegmentInfos segmentInfos; private ThreadPool threadPool; - private final String metadataFilename = RemoteSegmentStoreDirectory.getMetadataFilename(12, 23, 34, 1, 1, "node-1"); - - private final String metadataFilenameDup = RemoteSegmentStoreDirectory.getMetadataFilename(12, 23, 34, 2, 1, "node-2"); - private final String metadataFilename2 = RemoteSegmentStoreDirectory.getMetadataFilename(12, 13, 34, 1, 1, "node-1"); - private final String metadataFilename3 = RemoteSegmentStoreDirectory.getMetadataFilename(10, 38, 34, 1, 1, "node-1"); - private final String metadataFilename4 = RemoteSegmentStoreDirectory.getMetadataFilename(10, 36, 34, 1, 1, "node-1"); + private final String metadataFilename = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 12, + 23, + 34, + 1, + 1, + "node-1" + ); + + private final String metadataFilenameDup = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 12, + 23, + 34, + 2, + 1, + "node-2" + ); + private final String metadataFilename2 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 12, + 13, + 34, + 1, + 1, + "node-1" + ); + private final String metadataFilename3 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 10, + 38, + 34, + 1, + 1, + "node-1" + ); + private final String metadataFilename4 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 10, + 36, + 34, + 1, + 1, + "node-1" + ); @Before public void setup() throws IOException { @@ -473,7 +518,18 @@ public void testIsAcquiredException() throws IOException { private List getDummyMetadataFiles(int count) { List sortedMetadataFiles = new ArrayList<>(); for (int counter = 0; counter < count; counter++) { - sortedMetadataFiles.add(RemoteSegmentStoreDirectory.getMetadataFilename(counter, 23, 34, 1, 1, "node-1")); + sortedMetadataFiles.add( + RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + counter, + 23, + 34, + 1, + 1, + "node-1" + ) + ); } return sortedMetadataFiles; } @@ -752,7 +808,8 @@ public void testUploadMetadataEmpty() throws IOException { storeDirectory, 34L, indexShard.getLatestReplicationCheckpoint(), - "" + "", + false ) ); } @@ -799,7 +856,8 @@ public void testUploadMetadataNonEmpty() throws IOException { storeDirectory, generation, indexShard.getLatestReplicationCheckpoint(), - "" + "", + false ); verify(remoteMetadataDirectory).copyFrom( @@ -847,7 +905,8 @@ public void testUploadMetadataMissingSegment() throws IOException { storeDirectory, 12L, indexShard.getLatestReplicationCheckpoint(), - "" + "", + false ) ); verify(indexOutput).close(); @@ -1238,12 +1297,66 @@ private void indexDocs(int startDocId, int numberOfDocs) throws IOException { } public void testMetadataFileNameOrder() { - String file1 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 21, 23, 1, 1, ""); - String file2 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 38, 1, 1, ""); - String file3 = RemoteSegmentStoreDirectory.getMetadataFilename(18, 12, 26, 1, 1, ""); - String file4 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 10, 1, ""); - String file5 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 1, 1, ""); - String file6 = RemoteSegmentStoreDirectory.getMetadataFilename(15, 38, 32, 5, 1, ""); + String file1 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 15, + 21, + 23, + 1, + 1, + "" + ); + String file2 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 15, + 38, + 38, + 1, + 1, + "" + ); + String file3 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 18, + 12, + 26, + 1, + 1, + "" + ); + String file4 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 15, + 38, + 32, + 10, + 1, + "" + ); + String file5 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 15, + 38, + 32, + 1, + 1, + "" + ); + String file6 = RemoteSegmentStoreDirectory.getMetadataFilename( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR, + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 15, + 38, + 32, + 5, + 1, + "" + ); List actualList = new ArrayList<>(List.of(file1, file2, file3, file4, file5, file6)); actualList.sort(String::compareTo); From f675fab49d677303d6578b3c8ff3b7565c9f81fd Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Mon, 8 Apr 2024 15:41:45 +0530 Subject: [PATCH 3/9] Add missing setting in BUILT_IN_CLUSTER_SETTINGS Signed-off-by: Sachin Kale --- .../java/org/opensearch/common/settings/ClusterSettings.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 2904d49c224d7..c35a01c83b79a 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -732,7 +732,8 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_INDEX_SEGMENT_METADATA_RETENTION_MAX_COUNT_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, - RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING + RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING, + RemoteStoreSettings.CLUSTER_REMOTE_SEGMENT_SEPARATE_METADATA_SEGMENTINFOS_SETTING ) ) ); From 4f14894c1e882d54cd47fc2e3752072a95136981 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 9 Apr 2024 09:40:53 +0530 Subject: [PATCH 4/9] Run tests with random value for metadata segment infos separation Signed-off-by: Sachin Kale --- .../remotestore/RemoteStoreBaseIntegTestCase.java | 14 +++++++++----- .../org/opensearch/index/shard/IndexShard.java | 5 +++-- .../index/store/RemoteSegmentStoreDirectory.java | 6 ++++-- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java index d7ad0daa43524..9c9a64d47234f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java @@ -35,6 +35,7 @@ import org.opensearch.index.mapper.MapperService; import org.opensearch.index.shard.IndexShard; import org.opensearch.indices.IndicesService; +import org.opensearch.indices.RemoteStoreSettings; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.blobstore.BlobStoreRepository; @@ -135,14 +136,17 @@ protected Settings nodeSettings(int nodeOrdinal) { segmentRepoPath = randomRepoPath().toAbsolutePath(); translogRepoPath = randomRepoPath().toAbsolutePath(); } + Settings.Builder settingsBuilder = Settings.builder(); if (clusterSettingsSuppliedByTest) { - return Settings.builder().put(super.nodeSettings(nodeOrdinal)).build(); + settingsBuilder.put(super.nodeSettings(nodeOrdinal)); } else { - return Settings.builder() - .put(super.nodeSettings(nodeOrdinal)) - .put(remoteStoreClusterSettings(REPOSITORY_NAME, segmentRepoPath, REPOSITORY_2_NAME, translogRepoPath)) - .build(); + settingsBuilder.put(super.nodeSettings(nodeOrdinal)) + .put(remoteStoreClusterSettings(REPOSITORY_NAME, segmentRepoPath, REPOSITORY_2_NAME, translogRepoPath)); } + if (randomBoolean()) { + settingsBuilder.put(RemoteStoreSettings.CLUSTER_REMOTE_SEGMENT_SEPARATE_METADATA_SEGMENTINFOS_SETTING.getKey(), true); + } + return settingsBuilder.build(); } protected void setFailRate(String repoName, int value) throws ExecutionException, InterruptedException { diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 6496bc09a5462..1693a6bad96ba 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5051,7 +5051,7 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn final SegmentInfos infosSnapshot; if (remoteSegmentMetadata.getSegmentInfosBytes().length == 0) { List segmentInfosSnapshotFilenames = Arrays.stream(store.directory().listAll()) - .filter(file -> file.startsWith("segment_infos_snapshot")) + .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; infosSnapshot = SegmentInfos.readCommit(store.directory(), segmentInfosSnapshotFilenames.get(0)); @@ -5065,7 +5065,8 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn // delete any other commits, we want to start the engine only from a new commit made with the downloaded infos bytes. // Extra segments will be wiped on engine open. for (String file : List.of(store.directory().listAll())) { - if (file.startsWith(IndexFileNames.SEGMENTS) || file.startsWith("segment_infos_snapshot")) { + if (file.startsWith(IndexFileNames.SEGMENTS) + || file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) { store.deleteQuiet(file); } } diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 37ce03a892404..1ec2bcb18036b 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -80,6 +80,8 @@ public final class RemoteSegmentStoreDirectory extends FilterDirectory implement */ public static final String SEGMENT_NAME_UUID_SEPARATOR = "__"; + public static final String SEGMENT_INFOS_SNAPSHOT_PREFIX = "segment_infos_snapshot"; + /** * remoteDataDirectory is used to store segment files at path: cluster_UUID/index_UUID/shardId/segments/data */ @@ -633,7 +635,7 @@ public void uploadMetadata( } else { String segmentInfoSnapshotFilename = getMetadataFilename( MetadataFilenameUtils.SEPARATOR, - "segment_infos_snapshot", + SEGMENT_INFOS_SNAPSHOT_PREFIX, replicationCheckpoint.getPrimaryTerm(), segmentInfosSnapshot.getGeneration(), translogGeneration, @@ -656,7 +658,7 @@ public void uploadMetadata( segmentInfoSnapshotFilename, segmentInfoSnapshotFilename, segmentInfosSnapshotChecksum, - storeDirectory.fileLength(segmentInfosSnapshotChecksum) + storeDirectory.fileLength(segmentInfoSnapshotFilename) ); segmentInfosSnapshotMetadata.setWrittenByMajor(segmentInfosSnapshot.getCommitLuceneVersion().major); uploadedSegments.put(segmentInfoSnapshotFilename, segmentInfosSnapshotMetadata.toString()); From bb5c15ce253e77c97ff7273fac84f9bc4e508940 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 9 Apr 2024 09:49:36 +0530 Subject: [PATCH 5/9] Add changelog entry Signed-off-by: Sachin Kale --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a399e69cc5c3..ff55b0b9f26ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add an individual setting of rate limiter for segment replication ([#12959](https://github.com/opensearch-project/OpenSearch/pull/12959)) - [Streaming Indexing] Ensure support of the new transport by security plugin ([#13174](https://github.com/opensearch-project/OpenSearch/pull/13174)) - Add cluster setting to dynamically configure the buckets for filter rewrite optimization. ([#13179](https://github.com/opensearch-project/OpenSearch/pull/13179)) +- [Remote Store] Add support to separate segment infos snapshot from metadata ([#13114](https://github.com/opensearch-project/OpenSearch/pull/13114)) ### Dependencies - Bump `org.apache.commons:commons-configuration2` from 2.10.0 to 2.10.1 ([#12896](https://github.com/opensearch-project/OpenSearch/pull/12896)) From ab44a4ed2e31900df4492999b8eb8edc521f25aa Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 9 Apr 2024 13:03:17 +0530 Subject: [PATCH 6/9] Fix the replication failure issues Signed-off-by: Sachin Kale --- .../opensearch/index/shard/IndexShard.java | 6 ++++- .../store/RemoteSegmentStoreDirectory.java | 7 +++++- .../replication/SegmentReplicationTarget.java | 24 +++++++++++++++---- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 1693a6bad96ba..7a32730873bfc 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5054,7 +5054,11 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; - infosSnapshot = SegmentInfos.readCommit(store.directory(), segmentInfosSnapshotFilenames.get(0)); + infosSnapshot = SegmentInfos.readCommit( + store.directory(), + store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ), + remoteSegmentMetadata.getGeneration() + ); } else { infosSnapshot = store.buildSegmentInfos( remoteSegmentMetadata.getSegmentInfosBytes(), diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 1ec2bcb18036b..9e6865f5d4cb4 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -652,7 +652,12 @@ public void uploadMetadata( ) { segmentInfosSnapshot.write(segmentInfosIndexOutput); } - copyFrom(storeDirectory, segmentInfoSnapshotFilename, segmentInfoSnapshotFilename, IOContext.DEFAULT); + remoteDataDirectory.copyFrom( + storeDirectory, + segmentInfoSnapshotFilename, + segmentInfoSnapshotFilename, + IOContext.DEFAULT + ); String segmentInfosSnapshotChecksum = getChecksumOfLocalFile(storeDirectory, segmentInfoSnapshotFilename); UploadedSegmentMetadata segmentInfosSnapshotMetadata = new UploadedSegmentMetadata( segmentInfoSnapshotFilename, diff --git a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java index af764556b7549..3d5f97dbfb098 100644 --- a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java +++ b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java @@ -25,6 +25,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.index.shard.IndexShard; +import org.opensearch.index.store.RemoteSegmentStoreDirectory; import org.opensearch.index.store.Store; import org.opensearch.index.store.StoreFileMetadata; import org.opensearch.indices.recovery.MultiFileWriter; @@ -36,6 +37,7 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Set; @@ -283,8 +285,9 @@ private void updateFileRecoveryBytes(String fileName, long bytesRecovered) { private void finalizeReplication(CheckpointInfoResponse checkpointInfoResponse) throws OpenSearchCorruptionException { cancellableThreads.checkForCancel(); state.setStage(SegmentReplicationState.Stage.FINALIZE_REPLICATION); + byte[] segmentInfosBytes = checkpointInfoResponse.getInfosBytes(); // Handle empty SegmentInfos bytes for recovering replicas - if (checkpointInfoResponse.getInfosBytes() == null) { + if (segmentInfosBytes == null) { return; } Store store = null; @@ -292,10 +295,21 @@ private void finalizeReplication(CheckpointInfoResponse checkpointInfoResponse) store = store(); store.incRef(); multiFileWriter.renameAllTempFiles(); - final SegmentInfos infos = store.buildSegmentInfos( - checkpointInfoResponse.getInfosBytes(), - checkpointInfoResponse.getCheckpoint().getSegmentsGen() - ); + final SegmentInfos infos; + if (segmentInfosBytes.length == 0) { + List segmentInfosSnapshotFilenames = Arrays.stream(store.directory().listAll()) + .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) + .collect(Collectors.toList()); + assert segmentInfosSnapshotFilenames.size() == 1; + infos = SegmentInfos.readCommit( + store.directory(), + store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ), + checkpointInfoResponse.getCheckpoint().getSegmentsGen() + ); + store.deleteQuiet(segmentInfosSnapshotFilenames.get(0)); + } else { + infos = store.buildSegmentInfos(segmentInfosBytes, checkpointInfoResponse.getCheckpoint().getSegmentsGen()); + } indexShard.finalizeReplication(infos); } catch (CorruptIndexException | IndexFormatTooNewException | IndexFormatTooOldException ex) { // this is a fatal exception at this stage. From 9fcb9dae0489e6262b0a81af6ac277a88a8cea4c Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Thu, 11 Apr 2024 11:27:49 +0530 Subject: [PATCH 7/9] Make sure to close the index input of SegmentInfosSnapshot Signed-off-by: Sachin Kale --- .../java/org/opensearch/index/shard/IndexShard.java | 12 +++++++----- .../replication/SegmentReplicationTarget.java | 13 ++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 7a32730873bfc..d0edb75e9cec3 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5054,11 +5054,13 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; - infosSnapshot = SegmentInfos.readCommit( - store.directory(), - store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ), - remoteSegmentMetadata.getGeneration() - ); + try (ChecksumIndexInput segmentInfosInput = store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ)) { + infosSnapshot = SegmentInfos.readCommit( + store.directory(), + segmentInfosInput, + remoteSegmentMetadata.getGeneration() + ); + } } else { infosSnapshot = store.buildSegmentInfos( remoteSegmentMetadata.getSegmentInfosBytes(), diff --git a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java index 3d5f97dbfb098..3bbc124aae3a7 100644 --- a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java +++ b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java @@ -15,6 +15,7 @@ import org.apache.lucene.index.IndexFormatTooOldException; import org.apache.lucene.index.SegmentInfos; import org.apache.lucene.store.AlreadyClosedException; +import org.apache.lucene.store.ChecksumIndexInput; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.opensearch.OpenSearchCorruptionException; @@ -301,11 +302,13 @@ private void finalizeReplication(CheckpointInfoResponse checkpointInfoResponse) .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; - infos = SegmentInfos.readCommit( - store.directory(), - store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ), - checkpointInfoResponse.getCheckpoint().getSegmentsGen() - ); + try (ChecksumIndexInput segmentInfosInput = store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ)) { + infos = SegmentInfos.readCommit( + store.directory(), + segmentInfosInput, + checkpointInfoResponse.getCheckpoint().getSegmentsGen() + ); + } store.deleteQuiet(segmentInfosSnapshotFilenames.get(0)); } else { infos = store.buildSegmentInfos(segmentInfosBytes, checkpointInfoResponse.getCheckpoint().getSegmentsGen()); From 8ffb8e60dd050a15b94b4c873c0cb3f97cb7068d Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Fri, 12 Apr 2024 15:17:37 +0530 Subject: [PATCH 8/9] Fix Spotless errors Signed-off-by: Sachin Kale --- .../src/main/java/org/opensearch/index/shard/IndexShard.java | 5 ++++- .../indices/replication/SegmentReplicationTarget.java | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index d0edb75e9cec3..f40cbb9fef9bc 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5054,7 +5054,10 @@ public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, final Runn .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; - try (ChecksumIndexInput segmentInfosInput = store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ)) { + try ( + ChecksumIndexInput segmentInfosInput = store.directory() + .openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ) + ) { infosSnapshot = SegmentInfos.readCommit( store.directory(), segmentInfosInput, diff --git a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java index 3bbc124aae3a7..fc1341d7ffcd8 100644 --- a/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java +++ b/server/src/main/java/org/opensearch/indices/replication/SegmentReplicationTarget.java @@ -302,7 +302,10 @@ private void finalizeReplication(CheckpointInfoResponse checkpointInfoResponse) .filter(file -> file.startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX)) .collect(Collectors.toList()); assert segmentInfosSnapshotFilenames.size() == 1; - try (ChecksumIndexInput segmentInfosInput = store.directory().openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ)) { + try ( + ChecksumIndexInput segmentInfosInput = store.directory() + .openChecksumInput(segmentInfosSnapshotFilenames.get(0), IOContext.READ) + ) { infos = SegmentInfos.readCommit( store.directory(), segmentInfosInput, From d09738820ea615ced49aa90abacbc3f2abd2b45a Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Fri, 12 Apr 2024 15:38:53 +0530 Subject: [PATCH 9/9] Fix integ test Signed-off-by: Sachin Kale --- .../java/org/opensearch/remotestore/RemoteStoreIT.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java index b767ffff05e3a..9ffc816ee60bf 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java @@ -30,6 +30,7 @@ import org.opensearch.index.IndexSettings; import org.opensearch.index.shard.IndexShard; import org.opensearch.index.shard.IndexShardClosedException; +import org.opensearch.index.store.RemoteSegmentStoreDirectory; import org.opensearch.index.translog.Translog.Durability; import org.opensearch.indices.IndicesService; import org.opensearch.indices.RemoteStoreSettings; @@ -585,7 +586,9 @@ public void testFallbackToNodeToNodeSegmentCopy() throws Exception { try (Stream files = Files.list(segmentDataPath)) { files.forEach(p -> { try { - Files.delete(p); + if (p.getFileName().toString().startsWith(RemoteSegmentStoreDirectory.SEGMENT_INFOS_SNAPSHOT_PREFIX) == false) { + Files.delete(p); + } } catch (IOException e) { // Ignore }