-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix NPE in validateSearchableSnapshotRestorable when shard size is unavailable #19684
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 in validateSearchableSnapshotRestorable when shard size is unavailable #19684
Conversation
|
❌ Gradle check result for c2f7eb8: 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? |
|
The CI failure appears to be unrelated to my changes. The error is occurring in the Azure fixture Docker containers: This seems to be an infrastructure/flaky test issue rather than a problem with the NPE fix in RestoreService. |
8eacfc4 to
2baa1ab
Compare
|
❌ Gradle check result for 2baa1ab: 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? |
|
❌ Gradle check result for 2baa1ab: 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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this.
LGTM with a small rename suggestion. Can you please address this and rebase.
server/src/main/java/org/opensearch/snapshots/RestoreService.java
Outdated
Show resolved
Hide resolved
f762cb2 to
e76f134
Compare
|
❌ Gradle check result for e76f134: 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? |
3eb087d to
7ac1b80
Compare
7ac1b80 to
70f49a0
Compare
|
Apologies for the force push confusion. I accidentally included unrelated commits while cleaning up the merge commit, which notified many code owners unnecessarily. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19684 +/- ##
============================================
- Coverage 73.21% 73.21% -0.01%
+ Complexity 71564 71549 -15
============================================
Files 5785 5785
Lines 326759 326767 +8
Branches 47291 47294 +3
============================================
- Hits 239246 239238 -8
- Misses 68272 68284 +12
- Partials 19241 19245 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Sotaro Hikita <[email protected]>
597670a to
410142b
Compare
Signed-off-by: Andrew Ross <[email protected]>
|
❌ Gradle check result for b853af7: 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? |
Description
This PR fixes a NullPointerException that occurs when restoring remote snapshots when shard size information is not available in the ClusterInfo cache.
The
validateSearchableSnapshotRestorablemethod in RestoreService attempts to calculate the total size of existing remote snapshot shards.When
ClusterInfo.getShardSize()returns null (which is valid behavior when shard size info is unavailable), the stream operation mapToLong(Long::longValue) throws a NullPointerException.Related Issues
Resolves #19349
Check List
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.