From 912984826a922a4d692611a4c8c11fcc94b558ea Mon Sep 17 00:00:00 2001 From: Gaurav Bafna Date: Sat, 7 Oct 2023 10:38:29 +0530 Subject: [PATCH] Using hash of node id in metadata file names Signed-off-by: Gaurav Bafna --- .../org/opensearch/index/remote/RemoteStoreUtils.java | 7 +++---- .../index/store/RemoteSegmentStoreDirectory.java | 3 ++- .../translog/transfer/TranslogTransferMetadata.java | 2 +- .../opensearch/index/remote/RemoteStoreUtilsTests.java | 6 +++--- .../index/store/RemoteSegmentStoreDirectoryTests.java | 9 ++++++--- .../translog/transfer/TranslogTransferManagerTests.java | 8 ++++++-- 6 files changed, 21 insertions(+), 14 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 0ca9e0209c5ec..b4c33d781af86 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -82,7 +82,7 @@ public static String getSegmentName(String filename) { * @param fn Function to extract PrimaryTerm_Generation and Node Id from metadata file name . * fn returns null if node id is not part of the file name */ - static public void verifyNoMultipleWriters(List mdFiles, Function> fn) { + public static void verifyNoMultipleWriters(List mdFiles, Function> fn) { Map nodesByPrimaryTermAndGen = new HashMap<>(); mdFiles.forEach(mdFile -> { Tuple nodeIdByPrimaryTermAndGen = fn.apply(mdFile); @@ -91,10 +91,9 @@ static public void verifyNoMultipleWriters(List mdFiles, Function bmList = new LinkedList<>(); bmList.add(new PlainBlobMetadata(mdFilename, 1)); @@ -167,7 +167,7 @@ public void testVerifyMultipleWriters_Translog() throws InterruptedException { bmList = new LinkedList<>(); bmList.add(new PlainBlobMetadata(mdFilename, 1)); - TranslogTransferMetadata tm3 = new TranslogTransferMetadata(1, 1, 1, 2, "node-2"); + TranslogTransferMetadata tm3 = new TranslogTransferMetadata(1, 1, 1, 2, "node--2"); bmList.add(new PlainBlobMetadata(tm3.getFileName(), 1)); List finalBmList = bmList; assertThrows( 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 4a89b3c718f0b..0f44d5c3b2f53 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -67,6 +67,7 @@ import org.mockito.Mockito; import static org.opensearch.index.store.RemoteSegmentStoreDirectory.METADATA_FILES_TO_FETCH; +import static org.opensearch.index.store.RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR; import static org.opensearch.test.RemoteStoreTestUtils.createMetadataFileBytes; import static org.opensearch.test.RemoteStoreTestUtils.getDummyMetadata; import static org.hamcrest.CoreMatchers.is; @@ -213,9 +214,7 @@ public void testUploadedSegmentMetadataFromStringException() { } public void testGetPrimaryTermGenerationUuid() { - String[] filenameTokens = "abc__9223372036854775795__9223372036854775784__uuid_xyz".split( - RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR - ); + String[] filenameTokens = "abc__9223372036854775795__9223372036854775784__uuid_xyz".split(SEPARATOR); assertEquals(12, RemoteSegmentStoreDirectory.MetadataFilenameUtils.getPrimaryTerm(filenameTokens)); assertEquals(23, RemoteSegmentStoreDirectory.MetadataFilenameUtils.getGeneration(filenameTokens)); } @@ -1178,6 +1177,10 @@ public void testMetadataFileNameOrder() { actualList.sort(String::compareTo); assertEquals(List.of(file3, file2, file4, file6, file5, file1), actualList); + + long count = file1.chars().filter(ch -> ch == SEPARATOR.charAt(0)).count(); + // There should not be any `_` in mdFile name as it is used a separator . + assertEquals(14, count); } private static class WrapperIndexOutput extends IndexOutput { diff --git a/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java b/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java index a48dbdcdacb71..5deb4c83e52fc 100644 --- a/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java +++ b/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java @@ -44,6 +44,7 @@ import org.mockito.Mockito; +import static org.opensearch.index.translog.transfer.TranslogTransferMetadata.METADATA_SEPARATOR; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anySet; @@ -515,10 +516,13 @@ public void testMetadataConflict() throws InterruptedException { null, remoteTranslogTransferTracker ); - TranslogTransferMetadata tm = new TranslogTransferMetadata(1, 1, 1, 2, "node-1"); + TranslogTransferMetadata tm = new TranslogTransferMetadata(1, 1, 1, 2, "node--1"); String mdFilename = tm.getFileName(); + long count = mdFilename.chars().filter(ch -> ch == METADATA_SEPARATOR.charAt(0)).count(); + // There should not be any `_` in mdFile name as it is used a separator . + assertEquals(10, count); Thread.sleep(1); - TranslogTransferMetadata tm2 = new TranslogTransferMetadata(1, 1, 1, 2, "node-2"); + TranslogTransferMetadata tm2 = new TranslogTransferMetadata(1, 1, 1, 2, "node--2"); String mdFilename2 = tm2.getFileName(); doAnswer(invocation -> {