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

Fix NPE on restore searchable snapshot #13911

Merged
merged 4 commits into from
May 31, 2024

Conversation

bugmakerrrrrr
Copy link
Contributor

Description

Two changes here:

  1. fix the issue in [BUG] Searchable snapshot restore - null_pointer_exception #12966;
  2. make dynamic setting DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING take effect.

Related Issues

Resolves #12966

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • API changes companion pull request created.
  • 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.

Signed-off-by: panguixin <panguixin@bytedance.com>
@bugmakerrrrrr
Copy link
Contributor Author

@andrross @kotwanikunal please help to take a look when you get a chance

Copy link
Contributor

❌ Gradle check result for 60580f6: 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
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

Thanks @bugmakerrrrrr!

Let's add a changelog entry for this fix. Also it looks like there are spotless failures.

Signed-off-by: panguixin <panguixin@bytedance.com>
@andrross andrross added the backport 2.x Backport to 2.x branch label May 31, 2024
@andrross andrross added the v2.15.0 Issues and PRs related to version 2.15.0 label May 31, 2024
Copy link
Contributor

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

@andrross
Copy link
Member

Looks like this failure might be related to this change?

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderTests.testFileCacheRemoteShardsDecisions" -Dtests.seed=77C464051AC2A818 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sr-Latn-ME -Dtests.timezone=Pacific/Guam -Druntime.java=21

org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderTests > testFileCacheRemoteShardsDecisions FAILED
    java.lang.AssertionError: 
    Expected: <NO>
         but: was <YES>
        at __randomizedtesting.SeedInfo.seed([77C464051AC2A818:FE27C4DE8AE74CC9]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:964)
        at org.junit.Assert.assertThat(Assert.java:930)
        at org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderTests.testFileCacheRemoteShardsDecisions(DiskThresholdDeciderTests.java:462)

Signed-off-by: panguixin <panguixin@bytedance.com>
Copy link
Contributor

❌ Gradle check result for f8b1083: 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: Andrew Ross <andrross@amazon.com>
Copy link
Contributor

❕ Gradle check result for ed66558: UNSTABLE

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

Copy link

codecov bot commented May 31, 2024

Codecov Report

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

Project coverage is 71.57%. Comparing base (b15cb0c) to head (ed66558).
Report is 330 commits behind head on main.

Files Patch % Lines
...uting/allocation/decider/DiskThresholdDecider.java 66.66% 1 Missing and 1 partial ⚠️
...es/settings/put/TransportUpdateSettingsAction.java 0.00% 1 Missing ⚠️
...g/opensearch/cluster/routing/OperationRouting.java 0.00% 0 Missing and 1 partial ⚠️
.../java/org/opensearch/snapshots/RestoreService.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13911      +/-   ##
============================================
+ Coverage     71.42%   71.57%   +0.14%     
- Complexity    59978    61287    +1309     
============================================
  Files          4985     5065      +80     
  Lines        282275   288101    +5826     
  Branches      40946    41716     +770     
============================================
+ Hits         201603   206195    +4592     
- Misses        63999    64821     +822     
- Partials      16673    17085     +412     

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

@andrross andrross merged commit 9a87624 into opensearch-project:main May 31, 2024
30 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 31, 2024
Signed-off-by: panguixin <panguixin@bytedance.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 9a87624)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
andrross added a commit that referenced this pull request Jun 4, 2024
* Fix NPE on restore searchable snapshot (#13911)

Signed-off-by: panguixin <panguixin@bytedance.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 9a87624)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Add constant for compatiblity

Signed-off-by: Andrew Ross <andrross@amazon.com>

---------

Signed-off-by: panguixin <panguixin@bytedance.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
parv0201 pushed a commit to parv0201/OpenSearch that referenced this pull request Jun 10, 2024
Signed-off-by: panguixin <panguixin@bytedance.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch bug Something isn't working Search:Remote Search Storage:Snapshots v2.15.0 Issues and PRs related to version 2.15.0
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[BUG] Searchable snapshot restore - null_pointer_exception
2 participants