[Remote Store] Cleanup local-only translog files if no metadata in remote #12691

sachinpkale · 2024-03-15T09:34:11Z

Description

If we upload the translog files and before uploading translog metadata, process restarts (due to any reason) and the same node gets the shard assigned, the subsequent translog uploads fail with NPE.
Detailed analysis is provided in [BUG] [Remote Store] NullPointerException in translog upload if node drops before uploading the first metadata #12554
In this PR, while downloading translog, if remote does not contain a metadata file, we cleanup non-empty translog files on local.

Related Issues

[BUG] [Remote Store] NullPointerException in translog upload if node drops before uploading the first metadata #12554

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 · 2024-03-15T09:56:12Z

Compatibility status:

Checks if related components are compatible with change 57e07dc

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer.git]

github-actions · 2024-03-15T10:23:46Z

✅ Gradle check result for 7189b2b: SUCCESS

codecov · 2024-03-15T10:24:43Z

Codecov Report

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

Project coverage is 71.59%. Comparing base (b15cb0c) to head (57e07dc).
Report is 161 commits behind head on main.

Files	Patch %	Lines
...rg/opensearch/index/translog/RemoteFsTranslog.java	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #12691      +/-   ##
============================================
+ Coverage     71.42%   71.59%   +0.17%     
- Complexity    59978    60738     +760     
============================================
  Files          4985     5039      +54     
  Lines        282275   285353    +3078     
  Branches      40946    41329     +383     
============================================
+ Hits         201603   204290    +2687     
- Misses        63999    64216     +217     
- Partials      16673    16847     +174

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

server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java

github-actions · 2024-03-28T04:17:08Z

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

github-actions · 2024-03-28T05:37:38Z

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

github-actions · 2024-03-28T08:31:21Z

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

github-actions · 2024-04-04T05:59:02Z

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

github-actions · 2024-04-04T12:43:11Z

❕ Gradle check result for b56f9a2: UNSTABLE

TEST FAILURES:

      1 org.opensearch.remotestore.RemoteStoreStatsIT.testDownloadStatsCorrectnessSinglePrimarySingleReplica
      1 org.opensearch.gateway.RecoveryFromGatewayIT.testShardStoreFetchMultiNodeMultiIndexesUsingBatchAction

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

ashking94

Lgtm, left minor comments and questions.

server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java

server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java

Bukhtawar

Thanks Sachin, I am assuming the changes to reuse in-sync translog files on recovery will be handled differently

github-actions · 2024-04-08T06:06:17Z

❕ Gradle check result for 10b8770: UNSTABLE

TEST FAILURES:

      1 org.opensearch.gateway.RecoveryFromGatewayIT.testShardStoreFetchMultiNodeMultiIndexesUsingBatchAction

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

sachinpkale · 2024-04-09T03:17:39Z

Thanks Sachin, I am assuming the changes to reuse in-sync translog files on recovery will be handled differently

Yes Bukhtawar, this PR only handles a scenario where remote translog does not have any data uploaded. For in-sync translog files, this flow won't be triggered as remote has the files uploaded.

server/src/main/java/org/opensearch/index/translog/Translog.java

server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java

github-actions · 2024-04-12T06:32:50Z

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

github-actions · 2024-04-12T09:37:20Z

❌ Gradle check result for 41a11e5: 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: Sachin Kale <kalsac@amazon.com>

github-actions · 2024-04-15T08:10:53Z

✅ Gradle check result for 57e07dc: SUCCESS

opensearch-trigger-bot · 2024-04-17T09:06:16Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-12691-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1c208d581aa0597615e98e27894f1f4c2dde8e75
# Push it to GitHub
git push --set-upstream origin backport/backport-12691-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-12691-to-2.x.

…mote (opensearch-project#12691) Signed-off-by: Sachin Kale <kalsac@amazon.com>

…mote (#12691) (#13261) Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale added the skip-changelog label Mar 15, 2024

Bukhtawar reviewed Mar 26, 2024

View reviewed changes

server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java Outdated Show resolved Hide resolved

sachinpkale requested a review from Bukhtawar March 28, 2024 03:48

sachinpkale force-pushed the local-only-translog-cleanup branch from 02dae46 to b56f9a2 Compare April 4, 2024 11:49

ashking94 approved these changes Apr 6, 2024

View reviewed changes

Bukhtawar reviewed Apr 6, 2024

View reviewed changes

gbbafna reviewed Apr 10, 2024

View reviewed changes

server/src/main/java/org/opensearch/index/translog/Translog.java Show resolved Hide resolved

gbbafna reviewed Apr 11, 2024

View reviewed changes

server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java Outdated Show resolved Hide resolved

gbbafna approved these changes Apr 11, 2024

View reviewed changes

sachinpkale force-pushed the local-only-translog-cleanup branch from 45c5c0e to 41a11e5 Compare April 12, 2024 08:52

Sachin Kale added 5 commits April 15, 2024 12:35

Cleanup local-only translog files if no metadata in remote

98f3daa

Signed-off-by: Sachin Kale <kalsac@amazon.com>

Address PR comments

ce6eda1

Signed-off-by: Sachin Kale <kalsac@amazon.com>

Address PR comments

2a893f0

Signed-off-by: Sachin Kale <kalsac@amazon.com>

Address PR comments

2b99724

Signed-off-by: Sachin Kale <kalsac@amazon.com>

Change test as per the recent pathType change

57e07dc

Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale force-pushed the local-only-translog-cleanup branch from 41a11e5 to 57e07dc Compare April 15, 2024 07:05

gbbafna approved these changes Apr 17, 2024

View reviewed changes

gbbafna merged commit 1c208d5 into opensearch-project:main Apr 17, 2024
31 checks passed

gbbafna added the backport 2.x Backport to 2.x branch label Apr 17, 2024

opensearch-trigger-bot bot added the backport-failed label Apr 17, 2024

sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Apr 17, 2024

[Remote Store] Cleanup local-only translog files if no metadata in re…

8655c06

…mote (opensearch-project#12691) Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale mentioned this pull request Apr 17, 2024

[Backport 2.x][Remote Store] Cleanup local-only translog files if no metadata in remote (#12691) #13261

Merged

sachinpkale added a commit that referenced this pull request Apr 18, 2024

[Remote Store] Cleanup local-only translog files if no metadata in re…

a3f303b

…mote (#12691) (#13261) Signed-off-by: Sachin Kale <kalsac@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Remote Store] Cleanup local-only translog files if no metadata in remote #12691

[Remote Store] Cleanup local-only translog files if no metadata in remote #12691

sachinpkale commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024

codecov bot commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Apr 4, 2024

github-actions bot commented Apr 4, 2024

ashking94 left a comment

Bukhtawar left a comment

github-actions bot commented Apr 8, 2024

sachinpkale commented Apr 9, 2024

github-actions bot commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

github-actions bot commented Apr 15, 2024

opensearch-trigger-bot bot commented Apr 17, 2024

[Remote Store] Cleanup local-only translog files if no metadata in remote #12691

[Remote Store] Cleanup local-only translog files if no metadata in remote #12691

Conversation

sachinpkale commented Mar 15, 2024 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Mar 15, 2024 • edited Loading

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Mar 15, 2024

codecov bot commented Mar 15, 2024 • edited Loading

Codecov Report

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Mar 28, 2024

github-actions bot commented Apr 4, 2024

github-actions bot commented Apr 4, 2024

ashking94 left a comment

Choose a reason for hiding this comment

Bukhtawar left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2024

sachinpkale commented Apr 9, 2024

github-actions bot commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

github-actions bot commented Apr 15, 2024

opensearch-trigger-bot bot commented Apr 17, 2024

sachinpkale commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024 •

edited

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading