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.

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

added support for incremental download of translog files #16204

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rawahars
Copy link

@rawahars rawahars commented Oct 7, 2024

Description

Presently, the download workflow for remote backed storage works in a manner that causes the download of the same translog files multiple times, each time deleting all the older files before downloading them again. This causes significant wasted network bandwidth, along with the time taken for the shard to become active.

This change adds support for downloading the translog files incrementally and omitting the same if they are present locally.

Implementation

The key changes include-

Incremental Download Logic in RemoteFsTranslog:

  • The download logic now checks if the local translog files are present and have the same checksum as the remote files.
    • If the local and remote checksums match, the download is skipped for that generation.
    • If the checksums do not match, the download is performed for that generation.

Translog Footer Handling:

  • The TranslogFooter class has been added to handle the writing and reading of the translog footer, which contains the checksum of the translog data.
  • The footer is written when the TranslogWriter is closed, and the checksum is stored in the TranslogReader.
  • The TranslogReader is updated to handle reading the footer and ensuring that read requests do not overlap with the footer.

Generation-to-Checksum Mapping in TranslogTransferMetadata:

  • The TranslogTransferMetadata now includes a mapping between the translog generation and the corresponding checksum.
  • This mapping is used to compare the local and remote checksums during the incremental download process.
  • The TranslogTransferMetadataHandler is updated to read and write this generation-to-checksum mapping.

Testing

  • The RemoteFsTranslogTests class has been updated to include new test cases for the incremental download logic:
    • testIncrementalDownloadWithMatchingChecksum tests the scenario where the local and remote translog files have the same checksum, and the download is skipped.
    • testIncrementalDownloadWithDifferentChecksum tests the scenario where the local and remote translog files have different checksums, and only the missing generation is downloaded.
  • The TranslogFooterTests class has been added to verify the functionality of the TranslogFooter class, including writing and reading the footer.
  • The TranslogTransferMetadataHandlerTests class has been updated to include test cases for handling the generation-to-checksum map in the TranslogTransferMetadata.

Related Issues

One of the optimisations for #15277

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Contributor

github-actions bot commented Oct 7, 2024

❌ Gradle check result for cd08d89: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

github-actions bot commented Oct 7, 2024

✅ Gradle check result for f17a378: SUCCESS

Copy link

codecov bot commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 78.07018% with 25 lines in your changes missing coverage. Please review.

Project coverage is 72.02%. Comparing base (a81b868) to head (f17a378).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../org/opensearch/index/translog/TranslogFooter.java 70.58% 6 Missing and 4 partials ⚠️
.../org/opensearch/index/translog/TranslogReader.java 54.54% 2 Missing and 3 partials ⚠️
...rg/opensearch/index/translog/RemoteFsTranslog.java 84.00% 3 Missing and 1 partial ⚠️
.../org/opensearch/index/translog/TranslogWriter.java 66.66% 0 Missing and 4 partials ⚠️
...n/java/org/opensearch/index/translog/Translog.java 75.00% 1 Missing ⚠️
...dex/translog/transfer/TranslogTransferManager.java 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16204      +/-   ##
============================================
+ Coverage     71.94%   72.02%   +0.07%     
- Complexity    64612    64643      +31     
============================================
  Files          5298     5300       +2     
  Lines        301952   302060     +108     
  Branches      43627    43648      +21     
============================================
+ Hits         217247   217556     +309     
+ Misses        66884    66627     -257     
- Partials      17821    17877      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rawahars rawahars force-pushed the incremental_download_with_footer branch from f17a378 to b8f56ea Compare October 8, 2024 09:15
Copy link
Contributor

github-actions bot commented Oct 8, 2024

❌ Gradle check result for b8f56ea: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Presently, the download workflow for remote backed storage works in a manner which causes the download for same translog files multiple times, each time deleting all the older files before downloading them again. This causes significant wasted network bandwidth, along with the time taken for the shard to become active.

This change adds support for downloading the translog files incrementally and omitting the same if they are present locally.

Signed-off-by: Harsh Rawat <[email protected]>
This commit adds the unit tests applicable for the changes made to support incremental download of translog files. Primarily, the unit tests cover the changes in-
- TranslogFooter to write and read the footer of Translog
- RemoteFSTranslog to skip the download of locally present translog
- TranslogWriter to create reader with checksum upon close
- TranslogReader closeIntoReader functionality

Signed-off-by: Harsh Rawat <[email protected]>
@rawahars rawahars force-pushed the incremental_download_with_footer branch from 833b508 to 535bc28 Compare October 9, 2024 05:58
Copy link
Contributor

github-actions bot commented Oct 9, 2024

❌ Gradle check result for 535bc28: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants