Skip to content

Commit

Permalink
Remove timestamp from translog metadata filename for remote-backed in…
Browse files Browse the repository at this point in the history 
…dexes (opensearch-project#6262)

Signed-off-by: Ashish Singh <ssashish@amazon.com>
  • Loading branch information
@ashking94
ashking94 authored Feb 9, 2023
1 parent 4f5fd9c commit aabafa3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 34 deletions.
23 changes: 6 additions & 17 deletions server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ public class TranslogTransferMetadata {

private final long minTranslogGeneration;

private final long timeStamp;

private int count;

private final SetOnce<Map<String, String>> generationToPrimaryTermMapper = new SetOnce<>();
Expand All @@ -58,7 +56,6 @@ public TranslogTransferMetadata(long primaryTerm, long generation, long minTrans
this.primaryTerm = primaryTerm;
this.generation = generation;
this.minTranslogGeneration = minTranslogGeneration;
this.timeStamp = System.currentTimeMillis();
this.count = count;
}

Expand All @@ -68,7 +65,6 @@ public TranslogTransferMetadata(IndexInput indexInput) throws IOException {
this.primaryTerm = indexInput.readLong();
this.generation = indexInput.readLong();
this.minTranslogGeneration = indexInput.readLong();
this.timeStamp = indexInput.readLong();
this.generationToPrimaryTermMapper.set(indexInput.readMapOfStrings());
}

Expand Down Expand Up @@ -97,10 +93,7 @@ public Map<String, String> getGenerationToPrimaryTermMapper() {
}

public String getFileName() {
return String.join(
METADATA_SEPARATOR,
Arrays.asList(String.valueOf(primaryTerm), String.valueOf(generation), String.valueOf(timeStamp))
);
return String.join(METADATA_SEPARATOR, Arrays.asList(String.valueOf(primaryTerm), String.valueOf(generation)));
}

public byte[] createMetadataBytes() throws IOException {
Expand All @@ -123,24 +116,21 @@ public byte[] createMetadataBytes() throws IOException {

@Override
public int hashCode() {
return Objects.hash(primaryTerm, generation, timeStamp);
return Objects.hash(primaryTerm, generation);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
TranslogTransferMetadata other = (TranslogTransferMetadata) o;
return Objects.equals(this.primaryTerm, other.primaryTerm)
&& Objects.equals(this.generation, other.generation)
&& Objects.equals(this.timeStamp, other.timeStamp);
return Objects.equals(this.primaryTerm, other.primaryTerm) && Objects.equals(this.generation, other.generation);
}

private void write(DataOutput out) throws IOException {
out.writeLong(primaryTerm);
out.writeLong(generation);
out.writeLong(minTranslogGeneration);
out.writeLong(timeStamp);
if (generationToPrimaryTermMapper.get() != null) {
out.writeMapOfStrings(generationToPrimaryTermMapper.get());
} else {
Expand All @@ -151,12 +141,11 @@ private void write(DataOutput out) throws IOException {
private static class MetadataFilenameComparator implements Comparator<String> {
@Override
public int compare(String first, String second) {
// Format of metadata filename is <Primary Term>__<Generation>__<Timestamp>
// Format of metadata filename is <Primary Term>__<Generation>
String[] filenameTokens1 = first.split(METADATA_SEPARATOR);
String[] filenameTokens2 = second.split(METADATA_SEPARATOR);
// Here, we are not comparing only primary term and generation.
// Timestamp is not a good measure of comparison in case primary term and generation are same.
for (int i = 0; i < filenameTokens1.length - 1; i++) {
// Here, we are comparing only primary term and generation.
for (int i = 0; i < filenameTokens1.length; i++) {
if (filenameTokens1[i].equals(filenameTokens2[i]) == false) {
return Long.compare(Long.parseLong(filenameTokens1[i]), Long.parseLong(filenameTokens2[i]));
}
Expand Down
26 changes: 9 additions & 17 deletions ...er/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down Expand Up @@ -189,10 +190,10 @@ public void testReadMetadataSingleFile() throws IOException {
TranslogTransferManager translogTransferManager = new TranslogTransferManager(transferService, remoteBaseTransferPath, null);

// BlobPath does not have equals method, so we can't use the instance directly in when
when(transferService.listAll(any(BlobPath.class))).thenReturn(Sets.newHashSet("12__234__123456789"));
when(transferService.listAll(any(BlobPath.class))).thenReturn(Sets.newHashSet("12__234"));

TranslogTransferMetadata metadata = createTransferSnapshot().getTranslogTransferMetadata();
when(transferService.downloadBlob(any(BlobPath.class), eq("12__234__123456789"))).thenReturn(
when(transferService.downloadBlob(any(BlobPath.class), eq("12__234"))).thenReturn(
new ByteArrayInputStream(metadata.createMetadataBytes())
);

Expand All @@ -202,12 +203,10 @@ public void testReadMetadataSingleFile() throws IOException {
public void testReadMetadataMultipleFiles() throws IOException {
TranslogTransferManager translogTransferManager = new TranslogTransferManager(transferService, remoteBaseTransferPath, null);

when(transferService.listAll(any(BlobPath.class))).thenReturn(
Sets.newHashSet("12__234__56789", "12__235__56823", "12__233__56700")
);
when(transferService.listAll(any(BlobPath.class))).thenReturn(Sets.newHashSet("12__234", "12__235", "12__233"));

TranslogTransferMetadata metadata = createTransferSnapshot().getTranslogTransferMetadata();
when(transferService.downloadBlob(any(BlobPath.class), eq("12__235__56823"))).thenReturn(
when(transferService.downloadBlob(any(BlobPath.class), eq("12__235"))).thenReturn(
new ByteArrayInputStream(metadata.createMetadataBytes())
);

Expand All @@ -217,23 +216,16 @@ public void testReadMetadataMultipleFiles() throws IOException {
public void testReadMetadataException() throws IOException {
TranslogTransferManager translogTransferManager = new TranslogTransferManager(transferService, remoteBaseTransferPath, null);

when(transferService.listAll(any(BlobPath.class))).thenReturn(
Sets.newHashSet("12__234__56789", "12__235__56823", "12__233__56700")
);
when(transferService.listAll(any(BlobPath.class))).thenReturn(Sets.newHashSet("12__234", "12__235", "12__233"));

when(transferService.downloadBlob(any(BlobPath.class), eq("12__235__56823"))).thenThrow(new IOException("Something went wrong"));
when(transferService.downloadBlob(any(BlobPath.class), eq("12__235"))).thenThrow(new IOException("Something went wrong"));

assertNull(translogTransferManager.readMetadata());
}

public void testReadMetadataSamePrimaryTermGeneration() throws IOException {
TranslogTransferManager translogTransferManager = new TranslogTransferManager(transferService, remoteBaseTransferPath, null);

when(transferService.listAll(any(BlobPath.class))).thenReturn(
Sets.newHashSet("12__234__56789", "12__235__56823", "12__234__56700")
);

assertThrows(IllegalArgumentException.class, translogTransferManager::readMetadata);
List<String> metadataFiles = Arrays.asList("12__234", "12__235", "12__234");
assertThrows(IllegalArgumentException.class, () -> metadataFiles.sort(TranslogTransferMetadata.METADATA_FILENAME_COMPARATOR));
}

public void testDownloadTranslog() throws IOException {
Expand Down

0 comments on commit aabafa3

Please sign in to comment.