From 270e03a4dfc0856b3c301c2aa24def18ef8a6c4a Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 8 Jun 2023 16:52:28 +0530 Subject: [PATCH 01/16] Add Lucene version to UploadedSegmentMetadata Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) 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 d3e8d961337cc..ec9dc416b39c6 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -13,6 +13,8 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.index.SegmentCommitInfo; +import org.apache.lucene.index.SegmentInfo; import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.SegmentInfos; import org.apache.lucene.store.ByteBuffersDataOutput; @@ -22,6 +24,7 @@ import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.util.Version; import org.opensearch.ExceptionsHelper; import org.opensearch.action.ActionListener; import org.opensearch.common.UUIDs; @@ -32,6 +35,7 @@ import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; import org.opensearch.common.io.VersionedCodecStreamWrapper; +import org.opensearch.common.lucene.Lucene; import org.opensearch.common.lucene.store.ByteArrayIndexInput; import org.opensearch.common.util.ByteUtils; import org.opensearch.index.remote.RemoteStoreUtils; @@ -216,6 +220,7 @@ public static class UploadedSegmentMetadata { private final String uploadedFilename; private final String checksum; private final long length; + private Version writtenBy; UploadedSegmentMetadata(String originalFilename, String uploadedFilename, String checksum, long length) { this.originalFilename = originalFilename; @@ -226,7 +231,12 @@ public static class UploadedSegmentMetadata { @Override public String toString() { - return String.join(SEPARATOR, originalFilename, uploadedFilename, checksum, String.valueOf(length)); + String metadataStr = String.join(SEPARATOR, originalFilename, uploadedFilename, checksum, String.valueOf(length)); + if (writtenBy != null) { + metadataStr += SEPARATOR + writtenBy; + } + + return metadataStr; } public String getChecksum() { @@ -239,12 +249,25 @@ public long getLength() { public static UploadedSegmentMetadata fromString(String uploadedFilename) { String[] values = uploadedFilename.split(SEPARATOR); - return new UploadedSegmentMetadata(values[0], values[1], values[2], Long.parseLong(values[3])); + UploadedSegmentMetadata metadata = new UploadedSegmentMetadata(values[0], values[1], values[2], Long.parseLong(values[3])); + if (values.length == 5) { + metadata.setWrittenBy(values[4]); + } + + return metadata; } public String getOriginalFilename() { return originalFilename; } + + public void setWrittenBy(String writtenBy) { + this.writtenBy = Lucene.parseVersionLenient(writtenBy, Version.LATEST); + } + + public void setWrittenBy(Version writtenBy) { + this.writtenBy = writtenBy; + } } /** From 61a4f96febe15be84b364fc1a8ac2f518d777148 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 15 Jun 2023 23:52:57 +0530 Subject: [PATCH 02/16] Update tests and CHANGELOG Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 4 +-- .../RemoteSegmentStoreDirectoryTests.java | 35 ++++++++++++++++--- 2 files changed, 31 insertions(+), 8 deletions(-) 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 ec9dc416b39c6..81c5492ea21e6 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -250,9 +250,7 @@ public long getLength() { public static UploadedSegmentMetadata fromString(String uploadedFilename) { String[] values = uploadedFilename.split(SEPARATOR); UploadedSegmentMetadata metadata = new UploadedSegmentMetadata(values[0], values[1], values[2], Long.parseLong(values[3])); - if (values.length == 5) { - metadata.setWrittenBy(values[4]); - } + metadata.setWrittenBy(values[4]); return metadata; } 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 72d8c234d71c8..8395a14a4cc23 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -21,6 +21,7 @@ import org.apache.lucene.store.IndexOutput; import org.apache.lucene.store.OutputStreamIndexOutput; import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.util.Version; import org.junit.After; import org.junit.Before; import org.mockito.Mockito; @@ -124,14 +125,22 @@ public void testUploadedSegmentMetadataToString() { "123456", 1234 ); - assertEquals("abc::pqr::123456::1234", metadata.toString()); + metadata.setWrittenBy(Version.LATEST); + assertEquals("abc::pqr::123456::1234::" + Version.LATEST, metadata.toString()); } public void testUploadedSegmentMetadataFromString() { RemoteSegmentStoreDirectory.UploadedSegmentMetadata metadata = RemoteSegmentStoreDirectory.UploadedSegmentMetadata.fromString( - "_0.cfe::_0.cfe__uuidxyz::4567::372000" + "_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST + ); + assertEquals("_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST, metadata.toString()); + } + + public void testUploadedSegmentMetadataFromStringException() { + assertThrows( + ArrayIndexOutOfBoundsException.class, + () -> RemoteSegmentStoreDirectory.UploadedSegmentMetadata.fromString("_0.cfe::_0.cfe__uuidxyz::4567::372000") ); - assertEquals("_0.cfe::_0.cfe__uuidxyz::4567::372000", metadata.toString()); } public void testGetPrimaryTermGenerationUuid() { @@ -176,6 +185,12 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(1000, 5000) + "::" + randomIntBetween(512000, 1024000) + + "::" + + Version.MIN_SUPPORTED_MAJOR + + "." + + "0" + + "." + + "0" ); metadata.put( prefix + ".cfs", @@ -188,6 +203,12 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(1000, 5000) + "::" + randomIntBetween(512000, 1024000) + + "::" + + Version.MIN_SUPPORTED_MAJOR + + "." + + "0" + + "." + + "0" ); metadata.put( prefix + ".si", @@ -200,6 +221,8 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(1000, 5000) + "::" + randomIntBetween(512000, 1024000) + + "::" + + Version.LATEST ); metadata.put( "segments_" + commitGeneration, @@ -213,6 +236,8 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(1000, 5000) + "::" + randomIntBetween(1024, 5120) + + "::" + + Version.LATEST ); return metadata; } @@ -611,8 +636,8 @@ public void testContainsFile() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST); when(remoteMetadataDirectory.openInput(metadataFilename, IOContext.DEFAULT)).thenReturn(createMetadataFileBytes(metadata, 1, 5)); From bfd9b3c130db444ec4556143fe4b53edad44a695 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Fri, 16 Jun 2023 00:38:46 +0530 Subject: [PATCH 03/16] Pass RemoteSegmentMetadataHandlerTests tests Signed-off-by: Bhumika Saini --- .../remote/metadata/RemoteSegmentMetadataHandlerTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java b/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java index dc7940529f024..cdedd9b9e0072 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java +++ b/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java @@ -12,6 +12,7 @@ import org.apache.lucene.store.ByteBuffersDataOutput; import org.apache.lucene.store.ByteBuffersIndexOutput; import org.apache.lucene.store.OutputStreamIndexOutput; +import org.apache.lucene.util.Version; import org.junit.After; import org.junit.Before; import org.opensearch.cluster.metadata.IndexMetadata; @@ -134,6 +135,8 @@ private Map getDummyData() { + randomIntBetween(1000, 5000) + "::" + randomIntBetween(1024, 2048) + + "::" + + Version.LATEST ); expectedOutput.put( prefix + ".cfs", @@ -146,6 +149,8 @@ private Map getDummyData() { + randomIntBetween(1000, 5000) + "::" + randomIntBetween(1024, 2048) + + "::" + + Version.LATEST ); return expectedOutput; } From 7dd7a5f60d37f8c69cdd3ea6d4248b2be22b6d9d Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Wed, 21 Jun 2023 10:17:50 +0530 Subject: [PATCH 04/16] Remove CHANGELOG entry, throw exception if lucene version is missing Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 21 ++++++++++++++++++- .../RemoteSegmentStoreDirectoryTests.java | 20 +++++++++--------- 2 files changed, 30 insertions(+), 11 deletions(-) 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 81c5492ea21e6..d5ad497d6cb2c 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.SegmentCommitInfo; import org.apache.lucene.index.SegmentInfo; import org.apache.lucene.index.CorruptIndexException; @@ -603,10 +604,28 @@ public void uploadMetadata( ); try { try (IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT)) { + Map segmentToLuceneVersion = new HashMap<>(); + for (SegmentCommitInfo segmentCommitInfo : segmentInfosSnapshot) { + SegmentInfo info = segmentCommitInfo.info; + Set segFiles = info.files(); + for (String file : segFiles) { + segmentToLuceneVersion.put(file, info.getVersion()); + } + } + Map uploadedSegments = new HashMap<>(); for (String file : segmentFiles) { if (segmentsUploadedToRemoteStore.containsKey(file)) { - uploadedSegments.put(file, segmentsUploadedToRemoteStore.get(file).toString()); + UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); + if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { + metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); + } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { + metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); + } else { + throw new CorruptIndexException("Lucene version is missing for segment file", metadata.originalFilename); + } + + uploadedSegments.put(file, metadata.toString()); } else { throw new NoSuchFileException(file); } 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 8395a14a4cc23..adc2134ac79ae 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -725,8 +725,8 @@ public void testNoMetadataHeaderCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -748,8 +748,8 @@ public void testInvalidCodecHeaderCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -773,8 +773,8 @@ public void testHeaderMinVersionCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -798,8 +798,8 @@ public void testHeaderMaxVersionCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -823,8 +823,8 @@ public void testIncorrectChecksumCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512"); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024"); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST); BytesStreamOutput output = new BytesStreamOutput(); IndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); From dcb6dbd4227760d5886ec1680d29abed51ed9a05 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Mon, 3 Jul 2023 15:50:22 +0530 Subject: [PATCH 05/16] Fix RemoteSegmentStoreDirectoryTests Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 43 +++++++++++-------- .../RemoteSegmentStoreDirectoryTests.java | 36 ++++++++++++++-- 2 files changed, 59 insertions(+), 20 deletions(-) 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 d5ad497d6cb2c..d7655970f6020 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -221,6 +221,13 @@ public static class UploadedSegmentMetadata { private final String uploadedFilename; private final String checksum; private final long length; + + /** + * The Lucene version that wrote the original segment files. + * As part of the Lucene version compatibility check, this version information stored in the metadata + * will be used to skip downloading the segment files unnecessarily + * if they were written by an incompatible Lucene version. + */ private Version writtenBy; UploadedSegmentMetadata(String originalFilename, String uploadedFilename, String checksum, long length) { @@ -232,12 +239,7 @@ public static class UploadedSegmentMetadata { @Override public String toString() { - String metadataStr = String.join(SEPARATOR, originalFilename, uploadedFilename, checksum, String.valueOf(length)); - if (writtenBy != null) { - metadataStr += SEPARATOR + writtenBy; - } - - return metadataStr; + return String.join(SEPARATOR, originalFilename, uploadedFilename, checksum, String.valueOf(length), String.valueOf(writtenBy)); } public String getChecksum() { @@ -251,6 +253,10 @@ public long getLength() { public static UploadedSegmentMetadata fromString(String uploadedFilename) { String[] values = uploadedFilename.split(SEPARATOR); UploadedSegmentMetadata metadata = new UploadedSegmentMetadata(values[0], values[1], values[2], Long.parseLong(values[3])); + if (values.length < 5) { + logger.error("Lucene version is missing for UploadedSegmentMetadata: " + uploadedFilename); + } + metadata.setWrittenBy(values[4]); return metadata; @@ -613,17 +619,20 @@ public void uploadMetadata( } } - Map uploadedSegments = new HashMap<>(); - for (String file : segmentFiles) { - if (segmentsUploadedToRemoteStore.containsKey(file)) { - UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); - if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { - metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); - } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { - metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); - } else { - throw new CorruptIndexException("Lucene version is missing for segment file", metadata.originalFilename); - } + Map uploadedSegments = new HashMap<>(); + for (String file : segmentFiles) { + if (segmentsUploadedToRemoteStore.containsKey(file)) { + UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); + if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { + metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); + } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { + metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); + } else { + throw new CorruptIndexException( + "Lucene version is missing for segment file " + metadata.originalFilename, + metadata.originalFilename + ); + } uploadedSegments.put(file, metadata.toString()); } else { 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 adc2134ac79ae..096f529491fdc 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -674,7 +674,24 @@ public void testUploadMetadataEmpty() throws IOException { } public void testUploadMetadataNonEmpty() throws IOException { - populateMetadata(); + indexDocs(142364, 5); + flushShard(indexShard, true); + SegmentInfos segInfos = indexShard.store().readLastCommittedSegmentsInfo(); + long generation = segInfos.getGeneration(); + String latestMetadataFileName = "metadata__12__" + generation + "__abc"; + List metadataFiles = List.of(latestMetadataFileName); + when(remoteMetadataDirectory.listFilesByPrefix(RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX)).thenReturn( + metadataFiles + ); + Map> metadataFilenameContentMapping = Map.of( + latestMetadataFileName, + getDummyMetadata("_0", (int) generation) + ); + + when(remoteMetadataDirectory.openInput(latestMetadataFileName, IOContext.DEFAULT)).thenReturn( + createMetadataFileBytes(metadataFilenameContentMapping.get(latestMetadataFileName), generation, 12) + ); + remoteSegmentStoreDirectory.init(); Directory storeDirectory = mock(Directory.class); @@ -687,8 +704,7 @@ public void testUploadMetadataNonEmpty() throws IOException { indexOutput ); - Collection segmentFiles = List.of("_0.si", "_0.cfe", "_0.cfs", "segments_1"); - remoteSegmentStoreDirectory.uploadMetadata(segmentFiles, segmentInfos, storeDirectory, 12L, 34L); + remoteSegmentStoreDirectory.uploadMetadata(segInfos.files(true), segInfos, storeDirectory, 12L, 34L); verify(remoteMetadataDirectory).copyFrom( eq(storeDirectory), @@ -715,6 +731,14 @@ public void testUploadMetadataNonEmpty() throws IOException { } } + public void testUploadMetadataNoSegmentCommitInfos() throws IOException { + SegmentInfos segInfos = indexShard.store().readLastCommittedSegmentsInfo(); + int numSegCommitInfos = segInfos.size(); + assert numSegCommitInfos == 0 + : "For a fresh index, the number of SegmentCommitInfo instances associated with the SegmentInfos instance should be 0, but were found to be " + + numSegCommitInfos; + } + public void testNoMetadataHeaderCorruptIndexException() throws IOException { List metadataFiles = List.of(metadataFilename); when( @@ -972,6 +996,12 @@ public void testSegmentMetadataCurrentVersion() { assertEquals(RemoteSegmentMetadata.CURRENT_VERSION, 1); } + private void indexDocs(int startDocId, int numberOfDocs) throws IOException { + for (int i = startDocId; i < startDocId + numberOfDocs; i++) { + indexDoc(indexShard, "_doc", Integer.toString(i)); + } + } + public void testMetadataFileNameOrder() { String file1 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 21, 23, 1, 1); String file2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename(15, 38, 38, 1, 1); From 0a83f300e315cd3477bbf36f2e1f752197e44ba2 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Mon, 3 Jul 2023 17:34:55 +0530 Subject: [PATCH 06/16] Empty commit Signed-off-by: Bhumika Saini From d66113ab708908131026108c0ae6b9271000110b Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Mon, 3 Jul 2023 18:08:48 +0530 Subject: [PATCH 07/16] Refactor to use JUnit assertion Signed-off-by: Bhumika Saini --- .../index/store/RemoteSegmentStoreDirectoryTests.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 096f529491fdc..cdad9c5db5f1f 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -734,9 +734,12 @@ public void testUploadMetadataNonEmpty() throws IOException { public void testUploadMetadataNoSegmentCommitInfos() throws IOException { SegmentInfos segInfos = indexShard.store().readLastCommittedSegmentsInfo(); int numSegCommitInfos = segInfos.size(); - assert numSegCommitInfos == 0 - : "For a fresh index, the number of SegmentCommitInfo instances associated with the SegmentInfos instance should be 0, but were found to be " - + numSegCommitInfos; + assertEquals( + "For a fresh index, the number of SegmentCommitInfo instances associated with the SegmentInfos instance should be 0, but were found to be " + + numSegCommitInfos, + 0, + numSegCommitInfos + ); } public void testNoMetadataHeaderCorruptIndexException() throws IOException { From e917b914e39667ee6bf83b7dd58e38e70265dba6 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Tue, 11 Jul 2023 22:25:41 +0530 Subject: [PATCH 08/16] Pass testUploadMetadataNonEmpty with changes from main Signed-off-by: Bhumika Saini --- .../RemoteSegmentStoreDirectoryTests.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) 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 cdad9c5db5f1f..901992220ac3c 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -677,19 +677,24 @@ public void testUploadMetadataNonEmpty() throws IOException { indexDocs(142364, 5); flushShard(indexShard, true); SegmentInfos segInfos = indexShard.store().readLastCommittedSegmentsInfo(); + long primaryTerm = 12; + String primaryTermLong = RemoteStoreUtils.invertLong(primaryTerm); long generation = segInfos.getGeneration(); - String latestMetadataFileName = "metadata__12__" + generation + "__abc"; + String generationLong = RemoteStoreUtils.invertLong(generation); + String latestMetadataFileName = "metadata__" + primaryTermLong + "__" + generationLong + "__abc"; List metadataFiles = List.of(latestMetadataFileName); - when(remoteMetadataDirectory.listFilesByPrefix(RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX)).thenReturn( - metadataFiles - ); + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + 1 + ) + ).thenReturn(metadataFiles); Map> metadataFilenameContentMapping = Map.of( latestMetadataFileName, getDummyMetadata("_0", (int) generation) ); - when(remoteMetadataDirectory.openInput(latestMetadataFileName, IOContext.DEFAULT)).thenReturn( - createMetadataFileBytes(metadataFilenameContentMapping.get(latestMetadataFileName), generation, 12) + createMetadataFileBytes(metadataFilenameContentMapping.get(latestMetadataFileName), generation, primaryTerm) ); remoteSegmentStoreDirectory.init(); @@ -697,22 +702,17 @@ public void testUploadMetadataNonEmpty() throws IOException { Directory storeDirectory = mock(Directory.class); BytesStreamOutput output = new BytesStreamOutput(); IndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); + when(storeDirectory.createOutput(startsWith("metadata__" + primaryTermLong + "__" + generationLong), eq(IOContext.DEFAULT))) + .thenReturn(indexOutput); - String generation = RemoteStoreUtils.invertLong(segmentInfos.getGeneration()); - String primaryTerm = RemoteStoreUtils.invertLong(12); - when(storeDirectory.createOutput(startsWith("metadata__" + primaryTerm + "__" + generation), eq(IOContext.DEFAULT))).thenReturn( - indexOutput - ); - - remoteSegmentStoreDirectory.uploadMetadata(segInfos.files(true), segInfos, storeDirectory, 12L, 34L); + remoteSegmentStoreDirectory.uploadMetadata(segInfos.files(true), segInfos, storeDirectory, primaryTerm, generation); verify(remoteMetadataDirectory).copyFrom( eq(storeDirectory), - startsWith("metadata__" + primaryTerm + "__" + generation), - startsWith("metadata__" + primaryTerm + "__" + generation), + startsWith("metadata__" + primaryTermLong + "__" + generationLong), + startsWith("metadata__" + primaryTermLong + "__" + generationLong), eq(IOContext.DEFAULT) ); - VersionedCodecStreamWrapper streamWrapper = new VersionedCodecStreamWrapper<>( new RemoteSegmentMetadataHandler(), RemoteSegmentMetadata.CURRENT_VERSION, @@ -721,11 +721,9 @@ public void testUploadMetadataNonEmpty() throws IOException { RemoteSegmentMetadata remoteSegmentMetadata = streamWrapper.readStream( new ByteArrayIndexInput("expected", BytesReference.toBytes(output.bytes())) ); - Map actual = remoteSegmentStoreDirectory .getSegmentsUploadedToRemoteStore(); Map expected = remoteSegmentMetadata.getMetadata(); - for (String filename : expected.keySet()) { assertEquals(expected.get(filename).toString(), actual.get(filename).toString()); } From acb5c873f1bfcd3a8cb7bbe86ad4fc0522eb347f Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Wed, 12 Jul 2023 11:56:06 +0530 Subject: [PATCH 09/16] Rebase onto main Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) 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 d7655970f6020..6d51f0ef69a31 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -619,20 +619,20 @@ public void uploadMetadata( } } - Map uploadedSegments = new HashMap<>(); - for (String file : segmentFiles) { - if (segmentsUploadedToRemoteStore.containsKey(file)) { - UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); - if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { - metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); - } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { - metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); - } else { - throw new CorruptIndexException( - "Lucene version is missing for segment file " + metadata.originalFilename, - metadata.originalFilename - ); - } + Map uploadedSegments = new HashMap<>(); + for (String file : segmentFiles) { + if (segmentsUploadedToRemoteStore.containsKey(file)) { + UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); + if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { + metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); + } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { + metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); + } else { + throw new CorruptIndexException( + "Lucene version is missing for segment file " + metadata.originalFilename, + metadata.originalFilename + ); + } uploadedSegments.put(file, metadata.toString()); } else { From 8aa0a86f4704b026eb77de1a1a47beb01ddc250c Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 20 Jul 2023 13:41:17 +0530 Subject: [PATCH 10/16] Persist only Lucene major version in UploadedSegmentMetadata Signed-off-by: Bhumika Saini --- .../store/RemoteSegmentStoreDirectory.java | 74 +++++++++++++++---- .../RemoteSegmentStoreDirectoryTests.java | 22 +++++- 2 files changed, 80 insertions(+), 16 deletions(-) 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 6d51f0ef69a31..00597dbdb1336 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -36,7 +36,6 @@ import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; import org.opensearch.common.io.VersionedCodecStreamWrapper; -import org.opensearch.common.lucene.Lucene; import org.opensearch.common.lucene.store.ByteArrayIndexInput; import org.opensearch.common.util.ByteUtils; import org.opensearch.index.remote.RemoteStoreUtils; @@ -61,6 +60,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.zip.CRC32; @@ -223,12 +224,12 @@ public static class UploadedSegmentMetadata { private final long length; /** - * The Lucene version that wrote the original segment files. + * The Lucene major version that wrote the original segment files. * As part of the Lucene version compatibility check, this version information stored in the metadata * will be used to skip downloading the segment files unnecessarily * if they were written by an incompatible Lucene version. */ - private Version writtenBy; + private int writtenByMajor; UploadedSegmentMetadata(String originalFilename, String uploadedFilename, String checksum, long length) { this.originalFilename = originalFilename; @@ -239,7 +240,14 @@ public static class UploadedSegmentMetadata { @Override public String toString() { - return String.join(SEPARATOR, originalFilename, uploadedFilename, checksum, String.valueOf(length), String.valueOf(writtenBy)); + return String.join( + SEPARATOR, + originalFilename, + uploadedFilename, + checksum, + String.valueOf(length), + String.valueOf(writtenByMajor) + ); } public String getChecksum() { @@ -257,7 +265,7 @@ public static UploadedSegmentMetadata fromString(String uploadedFilename) { logger.error("Lucene version is missing for UploadedSegmentMetadata: " + uploadedFilename); } - metadata.setWrittenBy(values[4]); + metadata.setWrittenByMajor(Integer.parseInt(values[4])); return metadata; } @@ -266,12 +274,20 @@ public String getOriginalFilename() { return originalFilename; } - public void setWrittenBy(String writtenBy) { - this.writtenBy = Lucene.parseVersionLenient(writtenBy, Version.LATEST); - } - - public void setWrittenBy(Version writtenBy) { - this.writtenBy = writtenBy; + public void setWrittenByMajor(int writtenByMajor) { + if (writtenByMajor <= Version.LATEST.major && writtenByMajor >= Version.MIN_SUPPORTED_MAJOR) { + this.writtenByMajor = writtenByMajor; + } else { + throw new IllegalArgumentException( + "Lucene major version supplied (" + + writtenByMajor + + ") is incorrect. Should be between Version.LATEST (" + + Version.LATEST.major + + ") and Version.MIN_SUPPORTED_MAJOR (" + + Version.MIN_SUPPORTED_MAJOR + + ")." + ); + } } } @@ -610,12 +626,40 @@ public void uploadMetadata( ); try { try (IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT)) { - Map segmentToLuceneVersion = new HashMap<>(); + // TODO: The following regex could work incorrectly if both major and minor versions are double-digits. + // This is because the major and minor versions do not have a separator in the filename currently + // (Lucence). + // We may need to revisit this if the filename pattern is updated in future Lucene versions. + Pattern pattern = Pattern.compile("_\\d+_\\d+_Lucene(\\d+)\\d+_\\d+"); + Map segmentToLuceneVersion = new HashMap<>(); for (SegmentCommitInfo segmentCommitInfo : segmentInfosSnapshot) { SegmentInfo info = segmentCommitInfo.info; Set segFiles = info.files(); for (String file : segFiles) { - segmentToLuceneVersion.put(file, info.getVersion()); + segmentToLuceneVersion.put(file, info.getVersion().major); + } + + int dvUpdateLuceneMajorVersion = -1; + + Map> docValuesUpdatesFiles = segmentCommitInfo.getDocValuesUpdatesFiles(); + for (int key : docValuesUpdatesFiles.keySet()) { + for (String file : docValuesUpdatesFiles.get(key)) { + if (dvUpdateLuceneMajorVersion == -1) { + Matcher matcher = pattern.matcher(file); + if (matcher.find()) { + dvUpdateLuceneMajorVersion = Integer.parseInt(matcher.group(1)); + } else { + throw new CorruptIndexException("Unable to infer Lucene version for segment file " + file, file); + } + } + + segmentToLuceneVersion.put(file, dvUpdateLuceneMajorVersion); + } + + Set fieldInfosFiles = segmentCommitInfo.getFieldInfosFiles(); + for (String file : fieldInfosFiles) { + segmentToLuceneVersion.put(file, dvUpdateLuceneMajorVersion); + } } } @@ -624,9 +668,9 @@ public void uploadMetadata( if (segmentsUploadedToRemoteStore.containsKey(file)) { UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { - metadata.setWrittenBy(segmentToLuceneVersion.get(metadata.originalFilename)); + metadata.setWrittenByMajor(segmentToLuceneVersion.get(metadata.originalFilename)); } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { - metadata.setWrittenBy(segmentInfosSnapshot.getCommitLuceneVersion()); + metadata.setWrittenByMajor(segmentInfosSnapshot.getCommitLuceneVersion().major); } else { throw new CorruptIndexException( "Lucene version is missing for segment file " + metadata.originalFilename, 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 901992220ac3c..b8aee6a432c97 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -125,10 +125,30 @@ public void testUploadedSegmentMetadataToString() { "123456", 1234 ); - metadata.setWrittenBy(Version.LATEST); + metadata.setWrittenByMajor(Version.LATEST.major); assertEquals("abc::pqr::123456::1234::" + Version.LATEST, metadata.toString()); } + public void testUploadedSegmentMetadataToStringExceptionTooNew() { + RemoteSegmentStoreDirectory.UploadedSegmentMetadata metadata = new RemoteSegmentStoreDirectory.UploadedSegmentMetadata( + "abc", + "pqr", + "123456", + 1234 + ); + assertThrows(IllegalArgumentException.class, () -> metadata.setWrittenByMajor(Version.LATEST.major + 1)); + } + + public void testUploadedSegmentMetadataToStringExceptionTooOld() { + RemoteSegmentStoreDirectory.UploadedSegmentMetadata metadata = new RemoteSegmentStoreDirectory.UploadedSegmentMetadata( + "abc", + "pqr", + "123456", + 1234 + ); + assertThrows(IllegalArgumentException.class, () -> metadata.setWrittenByMajor(Version.LATEST.major - 2)); + } + public void testUploadedSegmentMetadataFromString() { RemoteSegmentStoreDirectory.UploadedSegmentMetadata metadata = RemoteSegmentStoreDirectory.UploadedSegmentMetadata.fromString( "_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST From 6d3eafeecc9685478e5cb89f52f57d7733cb335e Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 20 Jul 2023 14:10:43 +0530 Subject: [PATCH 11/16] Update UTs to only use Lucene major version in metadata Signed-off-by: Bhumika Saini --- .../RemoteSegmentStoreDirectoryTests.java | 42 ++++++++----------- .../RemoteSegmentMetadataHandlerTests.java | 4 +- 2 files changed, 19 insertions(+), 27 deletions(-) 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 b8aee6a432c97..fdac57c01151a 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -126,7 +126,7 @@ public void testUploadedSegmentMetadataToString() { 1234 ); metadata.setWrittenByMajor(Version.LATEST.major); - assertEquals("abc::pqr::123456::1234::" + Version.LATEST, metadata.toString()); + assertEquals("abc::pqr::123456::1234::" + Version.LATEST.major, metadata.toString()); } public void testUploadedSegmentMetadataToStringExceptionTooNew() { @@ -151,9 +151,9 @@ public void testUploadedSegmentMetadataToStringExceptionTooOld() { public void testUploadedSegmentMetadataFromString() { RemoteSegmentStoreDirectory.UploadedSegmentMetadata metadata = RemoteSegmentStoreDirectory.UploadedSegmentMetadata.fromString( - "_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST + "_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST.major ); - assertEquals("_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST, metadata.toString()); + assertEquals("_0.cfe::_0.cfe__uuidxyz::4567::372000::" + Version.LATEST.major, metadata.toString()); } public void testUploadedSegmentMetadataFromStringException() { @@ -207,10 +207,6 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(512000, 1024000) + "::" + Version.MIN_SUPPORTED_MAJOR - + "." - + "0" - + "." - + "0" ); metadata.put( prefix + ".cfs", @@ -225,10 +221,6 @@ private Map getDummyMetadata(String prefix, int commitGeneration + randomIntBetween(512000, 1024000) + "::" + Version.MIN_SUPPORTED_MAJOR - + "." - + "0" - + "." - + "0" ); metadata.put( prefix + ".si", @@ -242,7 +234,7 @@ private Map getDummyMetadata(String prefix, int commitGeneration + "::" + randomIntBetween(512000, 1024000) + "::" - + Version.LATEST + + Version.LATEST.major ); metadata.put( "segments_" + commitGeneration, @@ -257,7 +249,7 @@ private Map getDummyMetadata(String prefix, int commitGeneration + "::" + randomIntBetween(1024, 5120) + "::" - + Version.LATEST + + Version.LATEST.major ); return metadata; } @@ -656,8 +648,8 @@ public void testContainsFile() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST.major); when(remoteMetadataDirectory.openInput(metadataFilename, IOContext.DEFAULT)).thenReturn(createMetadataFileBytes(metadata, 1, 5)); @@ -770,8 +762,8 @@ public void testNoMetadataHeaderCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST.major); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -793,8 +785,8 @@ public void testInvalidCodecHeaderCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST.major); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -818,8 +810,8 @@ public void testHeaderMinVersionCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST.major); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -843,8 +835,8 @@ public void testHeaderMaxVersionCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::" + Version.LATEST.major); BytesStreamOutput output = new BytesStreamOutput(); OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); @@ -868,8 +860,8 @@ public void testIncorrectChecksumCorruptIndexException() throws IOException { ).thenReturn(metadataFiles); Map metadata = new HashMap<>(); - metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST); - metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST); + metadata.put("_0.cfe", "_0.cfe::_0.cfe__" + UUIDs.base64UUID() + "::1234::512::" + Version.LATEST.major); + metadata.put("_0.cfs", "_0.cfs::_0.cfs__" + UUIDs.base64UUID() + "::2345::1024::" + Version.LATEST.major); BytesStreamOutput output = new BytesStreamOutput(); IndexOutput indexOutput = new OutputStreamIndexOutput("segment metadata", "metadata output stream", output, 4096); diff --git a/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java b/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java index cdedd9b9e0072..2fee77ab563c0 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java +++ b/server/src/test/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataHandlerTests.java @@ -136,7 +136,7 @@ private Map getDummyData() { + "::" + randomIntBetween(1024, 2048) + "::" - + Version.LATEST + + Version.LATEST.major ); expectedOutput.put( prefix + ".cfs", @@ -150,7 +150,7 @@ private Map getDummyData() { + "::" + randomIntBetween(1024, 2048) + "::" - + Version.LATEST + + Version.LATEST.major ); return expectedOutput; } From c0279eb4e7ca73acf101aee6ecd2ad564bc47e17 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 20 Jul 2023 17:49:50 +0530 Subject: [PATCH 12/16] Modularize the logic to extract Lucene major version for segments Signed-off-by: Bhumika Saini --- .../index/remote/RemoteStoreUtils.java | 19 +++++ .../store/RemoteSegmentStoreDirectory.java | 76 +++++++++---------- 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index 1eeadfe228a45..4965456203210 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -8,7 +8,11 @@ package org.opensearch.index.remote; +import org.apache.lucene.index.CorruptIndexException; + import java.util.Arrays; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Utils for remote store @@ -48,4 +52,19 @@ public static long invertLong(String str) { } return Long.MAX_VALUE - num; } + + public static int getLuceneVersionForDocValuesUpdates(String filename) throws CorruptIndexException { + // TODO: The following regex could work incorrectly if both major and minor versions are double-digits. + // This is because the major and minor versions do not have a separator in the filename currently + // (Lucence). + // We may need to revisit this if the filename pattern is updated in future Lucene versions. + Pattern docValuesUpdatesFileNamePattern = Pattern.compile("_\\d+_\\d+_Lucene(\\d+)\\d+_\\d+"); + + Matcher matcher = docValuesUpdatesFileNamePattern.matcher(filename); + if (matcher.find()) { + return Integer.parseInt(matcher.group(1)); + } else { + throw new CorruptIndexException("Unable to infer Lucene version for segment file " + filename, filename); + } + } } 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 00597dbdb1336..05cb3a4ab5a43 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -60,8 +60,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.zip.CRC32; @@ -626,43 +624,7 @@ public void uploadMetadata( ); try { try (IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT)) { - // TODO: The following regex could work incorrectly if both major and minor versions are double-digits. - // This is because the major and minor versions do not have a separator in the filename currently - // (Lucence). - // We may need to revisit this if the filename pattern is updated in future Lucene versions. - Pattern pattern = Pattern.compile("_\\d+_\\d+_Lucene(\\d+)\\d+_\\d+"); - Map segmentToLuceneVersion = new HashMap<>(); - for (SegmentCommitInfo segmentCommitInfo : segmentInfosSnapshot) { - SegmentInfo info = segmentCommitInfo.info; - Set segFiles = info.files(); - for (String file : segFiles) { - segmentToLuceneVersion.put(file, info.getVersion().major); - } - - int dvUpdateLuceneMajorVersion = -1; - - Map> docValuesUpdatesFiles = segmentCommitInfo.getDocValuesUpdatesFiles(); - for (int key : docValuesUpdatesFiles.keySet()) { - for (String file : docValuesUpdatesFiles.get(key)) { - if (dvUpdateLuceneMajorVersion == -1) { - Matcher matcher = pattern.matcher(file); - if (matcher.find()) { - dvUpdateLuceneMajorVersion = Integer.parseInt(matcher.group(1)); - } else { - throw new CorruptIndexException("Unable to infer Lucene version for segment file " + file, file); - } - } - - segmentToLuceneVersion.put(file, dvUpdateLuceneMajorVersion); - } - - Set fieldInfosFiles = segmentCommitInfo.getFieldInfosFiles(); - for (String file : fieldInfosFiles) { - segmentToLuceneVersion.put(file, dvUpdateLuceneMajorVersion); - } - } - } - + Map segmentToLuceneVersion = getSegmentToLuceneVersion(segmentInfosSnapshot); Map uploadedSegments = new HashMap<>(); for (String file : segmentFiles) { if (segmentsUploadedToRemoteStore.containsKey(file)) { @@ -708,6 +670,42 @@ public void uploadMetadata( } } + /** + * Parses the provided SegmenttInfos to retrieve a mapping of segment files to the respective Lucene major version that wrote the segments + * @param segmentInfosSnapshot SegmenttInfos instance to parse + * @return Map of segment file name to the Lucene major version that wrote it + * @throws CorruptIndexException If the Lucene major version cannot be parsed for a segment file + */ + private Map getSegmentToLuceneVersion(SegmentInfos segmentInfosSnapshot) throws CorruptIndexException { + Map segmentToLuceneVersion = new HashMap<>(); + for (SegmentCommitInfo segmentCommitInfo : segmentInfosSnapshot) { + SegmentInfo info = segmentCommitInfo.info; + Set segFiles = info.files(); + for (String file : segFiles) { + segmentToLuceneVersion.put(file, info.getVersion().major); + } + + int docValuesUpdatesLuceneMajorVersion = -1; + Map> docValuesUpdatesFiles = segmentCommitInfo.getDocValuesUpdatesFiles(); + for (int key : docValuesUpdatesFiles.keySet()) { + for (String file : docValuesUpdatesFiles.get(key)) { + if (docValuesUpdatesLuceneMajorVersion == -1) { + docValuesUpdatesLuceneMajorVersion = RemoteStoreUtils.getLuceneVersionForDocValuesUpdates(file); + } + + segmentToLuceneVersion.put(file, docValuesUpdatesLuceneMajorVersion); + } + + Set fieldInfosFiles = segmentCommitInfo.getFieldInfosFiles(); + for (String file : fieldInfosFiles) { + segmentToLuceneVersion.put(file, docValuesUpdatesLuceneMajorVersion); + } + } + } + + return segmentToLuceneVersion; + } + /** * Try to delete file from local store. Fails silently on failures * @param filename: name of the file to be deleted From e1ce4e6fba86700b777c1ade69bc452b1e8346a8 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 20 Jul 2023 18:03:07 +0530 Subject: [PATCH 13/16] Add UTs for getLuceneVersionForDocValuesUpdates() Signed-off-by: Bhumika Saini --- .../org/opensearch/index/remote/RemoteStoreUtils.java | 6 ++++++ .../opensearch/index/remote/RemoteStoreUtilsTests.java | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index 4965456203210..e4466ca2cba5c 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -53,6 +53,12 @@ public static long invertLong(String str) { return Long.MAX_VALUE - num; } + /** + * Extracts the Lucene major version from the provided DocValuesUpdates file name + * @param filename DocValuesUpdates file name to parse + * @return Lucene major version that wrote the DocValuesUpdates file + * @throws CorruptIndexException If the Lucene major version cannot be inferred + */ public static int getLuceneVersionForDocValuesUpdates(String filename) throws CorruptIndexException { // TODO: The following regex could work incorrectly if both major and minor versions are double-digits. // This is because the major and minor versions do not have a separator in the filename currently 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 5b9135afb66f3..130ee86d405ea 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -8,6 +8,7 @@ package org.opensearch.index.remote; +import org.apache.lucene.index.CorruptIndexException; import org.opensearch.test.OpenSearchTestCase; public class RemoteStoreUtilsTests extends OpenSearchTestCase { @@ -38,4 +39,12 @@ public void testinvert() { assertEquals(num, RemoteStoreUtils.invertLong(RemoteStoreUtils.invertLong(num))); } } + + public void testGetLuceneVersionForDocValuesUpdates() throws CorruptIndexException { + assertEquals(9, RemoteStoreUtils.getLuceneVersionForDocValuesUpdates("_0_1_Lucene90_0.dvm")); + } + + public void testGetLuceneVersionForDocValuesUpdatesException() { + assertThrows(CorruptIndexException.class, () -> RemoteStoreUtils.getLuceneVersionForDocValuesUpdates("_0_1_Asserting_0.dvm")); + } } From 0e1adb453048980b8daeb4db98cb9461287d2a26 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Fri, 21 Jul 2023 17:24:43 +0530 Subject: [PATCH 14/16] Use .si file's Lucene version for files without version info Signed-off-by: Bhumika Saini --- .../index/remote/RemoteStoreUtils.java | 34 ++++++------- .../store/RemoteSegmentStoreDirectory.java | 49 +++++++------------ .../index/remote/RemoteStoreUtilsTests.java | 23 +++++++-- 3 files changed, 50 insertions(+), 56 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index e4466ca2cba5c..114d07589b0c0 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -8,11 +8,7 @@ package org.opensearch.index.remote; -import org.apache.lucene.index.CorruptIndexException; - import java.util.Arrays; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Utils for remote store @@ -54,23 +50,23 @@ public static long invertLong(String str) { } /** - * Extracts the Lucene major version from the provided DocValuesUpdates file name - * @param filename DocValuesUpdates file name to parse - * @return Lucene major version that wrote the DocValuesUpdates file - * @throws CorruptIndexException If the Lucene major version cannot be inferred + * Extracts the segment name from the provided segment file name + * @param filename Segment file name to parse + * @return Name of the segment that the segment file belongs to */ - public static int getLuceneVersionForDocValuesUpdates(String filename) throws CorruptIndexException { - // TODO: The following regex could work incorrectly if both major and minor versions are double-digits. - // This is because the major and minor versions do not have a separator in the filename currently - // (Lucence). - // We may need to revisit this if the filename pattern is updated in future Lucene versions. - Pattern docValuesUpdatesFileNamePattern = Pattern.compile("_\\d+_\\d+_Lucene(\\d+)\\d+_\\d+"); + public static String getSegmentName(String filename) { + // Segment file names follow patterns like "_0.cfe" or "_0_1_Lucene90_0.dvm". + // Here, the segment name is "_0", which is the set of characters + // starting with "_" until the next "_" or first ".". + int endIdx = filename.indexOf('_', 1); + if (endIdx == -1) { + endIdx = filename.indexOf('.'); + } - Matcher matcher = docValuesUpdatesFileNamePattern.matcher(filename); - if (matcher.find()) { - return Integer.parseInt(matcher.group(1)); - } else { - throw new CorruptIndexException("Unable to infer Lucene version for segment file " + filename, filename); + if (endIdx == -1) { + throw new IllegalArgumentException("Unable to infer segment name for segment file " + filename); } + + return filename.substring(0, endIdx); } } 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 05cb3a4ab5a43..8ee267cb67e68 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -16,7 +16,6 @@ import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.SegmentCommitInfo; import org.apache.lucene.index.SegmentInfo; -import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.SegmentInfos; import org.apache.lucene.store.ByteBuffersDataOutput; import org.apache.lucene.store.ByteBuffersIndexOutput; @@ -624,22 +623,12 @@ public void uploadMetadata( ); try { try (IndexOutput indexOutput = storeDirectory.createOutput(metadataFilename, IOContext.DEFAULT)) { - Map segmentToLuceneVersion = getSegmentToLuceneVersion(segmentInfosSnapshot); + Map segmentToLuceneVersion = getSegmentToLuceneVersion(segmentFiles, segmentInfosSnapshot); Map uploadedSegments = new HashMap<>(); for (String file : segmentFiles) { if (segmentsUploadedToRemoteStore.containsKey(file)) { UploadedSegmentMetadata metadata = segmentsUploadedToRemoteStore.get(file); - if (segmentToLuceneVersion.containsKey(metadata.originalFilename)) { - metadata.setWrittenByMajor(segmentToLuceneVersion.get(metadata.originalFilename)); - } else if (metadata.originalFilename.equals(segmentInfosSnapshot.getSegmentsFileName())) { - metadata.setWrittenByMajor(segmentInfosSnapshot.getCommitLuceneVersion().major); - } else { - throw new CorruptIndexException( - "Lucene version is missing for segment file " + metadata.originalFilename, - metadata.originalFilename - ); - } - + metadata.setWrittenByMajor(segmentToLuceneVersion.get(metadata.originalFilename)); uploadedSegments.put(file, metadata.toString()); } else { throw new NoSuchFileException(file); @@ -671,12 +660,13 @@ public void uploadMetadata( } /** - * Parses the provided SegmenttInfos to retrieve a mapping of segment files to the respective Lucene major version that wrote the segments - * @param segmentInfosSnapshot SegmenttInfos instance to parse - * @return Map of segment file name to the Lucene major version that wrote it - * @throws CorruptIndexException If the Lucene major version cannot be parsed for a segment file + * Parses the provided SegmentInfos to retrieve a mapping of the provided segment files to + * the respective Lucene major version that wrote the segments + * @param segmentFiles List of segment files for which the Lucene major version is needed + * @param segmentInfosSnapshot SegmentInfos instance to parse + * @return Map of the segment file to its Lucene major version */ - private Map getSegmentToLuceneVersion(SegmentInfos segmentInfosSnapshot) throws CorruptIndexException { + private Map getSegmentToLuceneVersion(Collection segmentFiles, SegmentInfos segmentInfosSnapshot) { Map segmentToLuceneVersion = new HashMap<>(); for (SegmentCommitInfo segmentCommitInfo : segmentInfosSnapshot) { SegmentInfo info = segmentCommitInfo.info; @@ -684,21 +674,16 @@ private Map getSegmentToLuceneVersion(SegmentInfos segmentInfos for (String file : segFiles) { segmentToLuceneVersion.put(file, info.getVersion().major); } + } - int docValuesUpdatesLuceneMajorVersion = -1; - Map> docValuesUpdatesFiles = segmentCommitInfo.getDocValuesUpdatesFiles(); - for (int key : docValuesUpdatesFiles.keySet()) { - for (String file : docValuesUpdatesFiles.get(key)) { - if (docValuesUpdatesLuceneMajorVersion == -1) { - docValuesUpdatesLuceneMajorVersion = RemoteStoreUtils.getLuceneVersionForDocValuesUpdates(file); - } - - segmentToLuceneVersion.put(file, docValuesUpdatesLuceneMajorVersion); - } - - Set fieldInfosFiles = segmentCommitInfo.getFieldInfosFiles(); - for (String file : fieldInfosFiles) { - segmentToLuceneVersion.put(file, docValuesUpdatesLuceneMajorVersion); + for (String file : segmentFiles) { + if (segmentToLuceneVersion.containsKey(file) == false) { + if (file.equals(segmentInfosSnapshot.getSegmentsFileName())) { + segmentToLuceneVersion.put(file, segmentInfosSnapshot.getCommitLuceneVersion().major); + } else { + // Fallback to the Lucene major version of the respective segment's .si file + String segmentInfoFileName = RemoteStoreUtils.getSegmentName(file) + ".si"; + segmentToLuceneVersion.put(file, segmentToLuceneVersion.get(segmentInfoFileName)); } } } 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 130ee86d405ea..9afa75dd601b2 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -8,7 +8,6 @@ package org.opensearch.index.remote; -import org.apache.lucene.index.CorruptIndexException; import org.opensearch.test.OpenSearchTestCase; public class RemoteStoreUtilsTests extends OpenSearchTestCase { @@ -40,11 +39,25 @@ public void testinvert() { } } - public void testGetLuceneVersionForDocValuesUpdates() throws CorruptIndexException { - assertEquals(9, RemoteStoreUtils.getLuceneVersionForDocValuesUpdates("_0_1_Lucene90_0.dvm")); + public void testGetSegmentNameForCfeFile() { + assertEquals("_foo", RemoteStoreUtils.getSegmentName("_foo.cfe")); } - public void testGetLuceneVersionForDocValuesUpdatesException() { - assertThrows(CorruptIndexException.class, () -> RemoteStoreUtils.getLuceneVersionForDocValuesUpdates("_0_1_Asserting_0.dvm")); + public void testGetSegmentNameForDvmFile() { + assertEquals("_bar", RemoteStoreUtils.getSegmentName("_bar_1_Lucene90_0.dvm")); + } + + public void testGetSegmentNameWeirdSegmentNameOnlyUnderscore() { + // Validate behaviour when segment name contains delimiters only + assertEquals("_", RemoteStoreUtils.getSegmentName("_.dvm")); + } + + public void testGetSegmentNameUnderscoreDelimiterOverrides() { + // Validate behaviour when segment name contains delimiters only + assertEquals("_", RemoteStoreUtils.getSegmentName("___.dvm")); + } + + public void testGetSegmentNameException() { + assertThrows(IllegalArgumentException.class, () -> RemoteStoreUtils.getSegmentName("dvd")); } } From de3324a19a8eb0f8a2aa58d9a4874bfe7754cee1 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Fri, 21 Jul 2023 17:32:34 +0530 Subject: [PATCH 15/16] Fix segment names in testUploadMetadataEmpty() Signed-off-by: Bhumika Saini --- .../index/store/RemoteSegmentStoreDirectoryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fdac57c01151a..7c765cf5df0be 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -678,7 +678,7 @@ public void testUploadMetadataEmpty() throws IOException { IndexOutput indexOutput = mock(IndexOutput.class); when(storeDirectory.createOutput(startsWith("metadata__12__o"), eq(IOContext.DEFAULT))).thenReturn(indexOutput); - Collection segmentFiles = List.of("s1", "s2", "s3"); + Collection segmentFiles = List.of("_s1.si", "_s1.cfe", "_s3.cfs"); assertThrows( NoSuchFileException.class, () -> remoteSegmentStoreDirectory.uploadMetadata(segmentFiles, segmentInfos, storeDirectory, 12L, 34L) From c795f71dfb749193ded2dc93512e6cda1e941f1c Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Fri, 21 Jul 2023 18:08:33 +0530 Subject: [PATCH 16/16] Empty commit Signed-off-by: Bhumika Saini