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

Upload translog.ckp file data as object metadata with translog.tlog file for s3 remote store #13272

Closed

Conversation

skumawat2025
Copy link
Contributor

@skumawat2025 skumawat2025 commented Apr 17, 2024

Description

This PR is a sub task of feature request #13022. And is being tracked here #13091.

This PR include below changes -

  1. The translog.ckp file's data is now stored as object metadata associated with the translog.tlog file when uploading to an S3 remote storage.
  2. The translog download flow has been updated to handle mixed index scenarios. In cases where translog files were previously uploaded to the remote storage in different formats (separate translog.tlog and translog.ckp files) as well as the new upload format (checkpoint file as header/metadata to translog.tlog file)
  3. Deletion of translog files

Related Issues

Resolves #13094

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request Storage:Performance v2.14.0 labels Apr 17, 2024
Copy link
Contributor

❌ Gradle check result for b22a58d: 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?

Signed-off-by: Sandeep Kumawat <[email protected]>
Copy link
Contributor

❌ Gradle check result for fde9788: 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

❌ Gradle check result for 57326eb: 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

❌ Gradle check result for 23f4a64: 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

❌ Gradle check result for e69b2b4: 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

❌ Gradle check result for 5dc4ab2: 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?

@skumawat2025 skumawat2025 changed the title Upload translog.ckp file data as metadata to translog.tlog file for s3 remote store Upload translog.ckp file data as object metadata to translog.tlog file for s3 remote store Apr 19, 2024
@skumawat2025 skumawat2025 changed the title Upload translog.ckp file data as object metadata to translog.tlog file for s3 remote store Upload translog.ckp file data as object metadata with translog.tlog file for s3 remote store Apr 19, 2024
@skumawat2025 skumawat2025 self-assigned this Apr 21, 2024
Signed-off-by: Sandeep Kumawat <[email protected]>
Copy link

codecov bot commented Apr 21, 2024

Codecov Report

Attention: Patch coverage is 85.23316% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 71.55%. Comparing base (b15cb0c) to head (bd58d92).
Report is 275 commits behind head on main.

❗ Current head bd58d92 differs from pull request most recent head fd5450f. Consider uploading reports for the commit fd5450f to get more accurate results

Files Patch % Lines
...sfer/TranslogCkpAsMetadataFileTransferManager.java 86.76% 4 Missing and 5 partials ⚠️
.../translog/transfer/TranslogCheckpointSnapshot.java 76.47% 8 Missing ⚠️
...slog/transfer/TranslogCkpFilesTransferManager.java 87.27% 1 Missing and 6 partials ⚠️
...slog/transfer/TranslogCkpFilesTransferTracker.java 88.88% 3 Missing and 4 partials ⚠️
.../org/opensearch/index/remote/RemoteStoreUtils.java 50.00% 0 Missing and 4 partials ⚠️
...ex/translog/transfer/BlobStoreTransferService.java 20.00% 4 Missing ⚠️
...ex/remote/RemoteMigrationIndexMetadataUpdater.java 62.50% 0 Missing and 3 partials ⚠️
...ensearch/index/translog/transfer/FileSnapshot.java 66.66% 3 Missing ⚠️
...rg/opensearch/index/translog/RemoteFsTranslog.java 83.33% 1 Missing and 1 partial ⚠️
...va/org/opensearch/indices/RemoteStoreSettings.java 66.66% 2 Missing ⚠️
... and 8 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13272      +/-   ##
============================================
+ Coverage     71.42%   71.55%   +0.13%     
- Complexity    59978    61172    +1194     
============================================
  Files          4985     5060      +75     
  Lines        282275   287403    +5128     
  Branches      40946    41628     +682     
============================================
+ Hits         201603   205650    +4047     
- Misses        63999    64830     +831     
- Partials      16673    16923     +250     

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

Copy link
Contributor

github-actions bot commented May 7, 2024

❌ Gradle check result for 7437c73: 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?

@skumawat2025 skumawat2025 force-pushed the storage-issue-13094 branch from 7437c73 to 06a730d Compare May 8, 2024 06:04
Copy link
Contributor

github-actions bot commented May 8, 2024

❌ Gradle check result for 06a730d: null

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?

Signed-off-by: Sandeep Kumawat <[email protected]>
@skumawat2025 skumawat2025 force-pushed the storage-issue-13094 branch from 06a730d to 828f8e3 Compare May 8, 2024 11:29
Copy link
Contributor

github-actions bot commented May 8, 2024

❌ Gradle check result for 828f8e3: 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 May 8, 2024

✅ Gradle check result for e058a92: SUCCESS

Copy link
Contributor

❌ Gradle check result for a993ee5: 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?

@skumawat2025 skumawat2025 force-pushed the storage-issue-13094 branch from a993ee5 to b5cdfdd Compare May 10, 2024 13:05
Copy link
Contributor

❌ Gradle check result for b5cdfdd: 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?

@skumawat2025 skumawat2025 force-pushed the storage-issue-13094 branch from b5cdfdd to 530277c Compare May 10, 2024 13:26
Copy link
Contributor

❌ Gradle check result for 530277c: 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

❌ Gradle check result for 2d6a296: 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?

Signed-off-by: Sandeep Kumawat <[email protected]>
Copy link
Contributor

✅ Gradle check result for bd58d92: SUCCESS

*
* @opensearch.internal
*/
public class TranslogCkpAsMetadataFileTransferManager extends TranslogTransferManager {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the naming of these files, it looks to specific to the PR and not generic

Comment on lines 103 to 108
if (isUploaded(tlogFileName) == false) {
recordFileContentLength(tlogFileName, file::getTranslogFileContentLength);
}
if (isUploaded(ckpFileName) == false) {
recordFileContentLength(ckpFileName, file::getCheckpointFileContentLength);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not checksum?

Copy link
Collaborator

@Bukhtawar Bukhtawar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are way too many abstractions with too specific implementations. We just need a metadata provider abstraction instead and simplify this PR

@ashking94 ashking94 force-pushed the storage-issue-13094 branch from 46bffbe to fd5450f Compare May 13, 2024 09:05
Copy link
Contributor

❌ Gradle check result for 46bffbe: 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

❌ Gradle check result for fd5450f: 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?

@sachinpkale
Copy link
Member

This is moved to: #13637

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request skip-changelog Storage:Performance Storage:Remote v2.15.0 Issues and PRs related to version 2.15.0
Projects
Status: ✅ Done
5 participants