From b2a836b30b6cf88032f4a6acd1d16577e260af2d Mon Sep 17 00:00:00 2001 From: Ashish Date: Sat, 7 Oct 2023 17:49:26 +0530 Subject: [PATCH] [Backport 2.x] [Remote Store] Using hash of node id in metadata file names (#10491) * [Remote Store] Using hash of node id in metadata file names (#10480) Signed-off-by: Gaurav Bafna Signed-off-by: Ashish Singh * Fix failing testGetPrimaryTermAndGeneration in TranslogTransferManagerTests (#10490) Signed-off-by: Ashish Singh --------- Signed-off-by: Gaurav Bafna Signed-off-by: Ashish Singh Co-authored-by: Gaurav Bafna <85113518+gbbafna@users.noreply.github.com> --- .../index/remote/RemoteStoreUtils.java | 7 +++---- .../store/RemoteSegmentStoreDirectory.java | 3 ++- .../transfer/TranslogTransferMetadata.java | 2 +- .../index/remote/RemoteStoreUtilsTests.java | 6 +++--- .../RemoteSegmentStoreDirectoryTests.java | 9 ++++++--- .../transfer/TranslogTransferManagerTests.java | 18 ++++++++++++++---- 6 files changed, 29 insertions(+), 16 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..af596e7df02c2 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 @@ -39,11 +39,14 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; 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; @@ -503,8 +506,12 @@ private void assertTlogCkpDownloadStats() { } public void testGetPrimaryTermAndGeneration() { - String tm = new TranslogTransferMetadata(1, 2, 1, 2, "node-1").getFileName(); - assertEquals(new Tuple<>(new Tuple<>(1L, 2L), "node-1"), TranslogTransferMetadata.getNodeIdByPrimaryTermAndGeneration(tm)); + String nodeId = UUID.randomUUID().toString(); + String tm = new TranslogTransferMetadata(1, 2, 1, 2, nodeId).getFileName(); + Tuple, String> actualOutput = TranslogTransferMetadata.getNodeIdByPrimaryTermAndGeneration(tm); + assertEquals(1L, (long) (actualOutput.v1().v1())); + assertEquals(2L, (long) (actualOutput.v1().v2())); + assertEquals(String.valueOf(Objects.hash(nodeId)), actualOutput.v2()); } public void testMetadataConflict() throws InterruptedException { @@ -515,10 +522,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 -> {