Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.11] [Remote Store] Using hash of node id in metadata file names #10483

Closed
wants to merge 1 commit into from
Closed

[Backport 2.11] [Remote Store] Using hash of node id in metadata file names #10483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> mdFiles, Function<String, Tuple<String, String>> fn) {
public static void verifyNoMultipleWriters(List<String> mdFiles, Function<String, Tuple<String, String>> fn) {
Map<String, String> nodesByPrimaryTermAndGen = new HashMap<>();
mdFiles.forEach(mdFile -> {
Tuple<String, String> nodeIdByPrimaryTermAndGen = fn.apply(mdFile);
Expand All @@ -91,10 +91,9 @@ static public void verifyNoMultipleWriters(List<String> mdFiles, Function<String
&& (!nodesByPrimaryTermAndGen.get(nodeIdByPrimaryTermAndGen.v1()).equals(nodeIdByPrimaryTermAndGen.v2()))) {
throw new IllegalStateException(
"Multiple metadata files from different nodes"
+ "having same primary term and generations "
+ nodeIdByPrimaryTermAndGen.v1()
+ " and "
+ nodeIdByPrimaryTermAndGen.v2()
+ "having same primary term and generations detected"
+ " detected "
);
}
nodesByPrimaryTermAndGen.put(nodeIdByPrimaryTermAndGen.v1(), nodeIdByPrimaryTermAndGen.v2());
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -332,7 +333,7 @@ public static String getMetadataFilename(
RemoteStoreUtils.invertLong(generation),
RemoteStoreUtils.invertLong(translogGeneration),
RemoteStoreUtils.invertLong(uploadCounter),
nodeId,
String.valueOf(Objects.hash(nodeId)),
RemoteStoreUtils.invertLong(System.currentTimeMillis()),
String.valueOf(metadataVersion)
);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public String getFileName() {
RemoteStoreUtils.invertLong(primaryTerm),
RemoteStoreUtils.invertLong(generation),
RemoteStoreUtils.invertLong(createdAt),
nodeId,
String.valueOf(Objects.hash(nodeId)),
String.valueOf(CURRENT_VERSION)
)
);
Expand Down
6 changes: 3 additions & 3 deletions server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ public void testVerifyMultipleWriters_Segment() {
}

public void testVerifyMultipleWriters_Translog() throws InterruptedException {
TranslogTransferMetadata tm = new TranslogTransferMetadata(1, 1, 1, 2, "node-1");
TranslogTransferMetadata tm = new TranslogTransferMetadata(1, 1, 1, 2, "node--1");
String mdFilename = tm.getFileName();
Thread.sleep(1);
TranslogTransferMetadata tm2 = new TranslogTransferMetadata(1, 1, 1, 2, "node-1");
TranslogTransferMetadata tm2 = new TranslogTransferMetadata(1, 1, 1, 2, "node--1");
String mdFilename2 = tm2.getFileName();
List<BlobMetadata> bmList = new LinkedList<>();
bmList.add(new PlainBlobMetadata(mdFilename, 1));
Expand All @@ -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<BlobMetadata> finalBmList = bmList;
assertThrows(
Expand Down
9 changes: 6 additions & 3 deletions server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions ...er/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 -> {
Expand Down
Loading