-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[SnapshotV2] Add snapshot size to status response for shallow V2 snapshots #15573
base: main
Are you sure you want to change the base?
Conversation
b2bda82
to
39248a5
Compare
❌ Gradle check result for b2bda82: 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 39248a5: 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? |
39248a5
to
002abda
Compare
❕ Gradle check result for 002abda: 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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15573 +/- ##
============================================
+ Coverage 71.92% 72.01% +0.09%
- Complexity 64400 64446 +46
============================================
Files 5281 5281
Lines 300995 301045 +50
Branches 43479 43492 +13
============================================
+ Hits 216491 216802 +311
+ Misses 66793 66432 -361
- Partials 17711 17811 +100 ☔ View full report in Codecov by Sentry. |
Flaky test #14408 |
@@ -96,7 +99,12 @@ public class SnapshotStatus implements ToXContentObject, Writeable { | |||
includeGlobalState = in.readOptionalBoolean(); | |||
final long startTime = in.readLong(); | |||
final long time = in.readLong(); | |||
updateShardStats(startTime, time); | |||
if (in.getVersion().onOrAfter(Version.CURRENT)) { |
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.
While checking this in 'main', please could you help add a TODO comment with the specific version this needs to be replaced with in the follow-up PR? Applicable for all places we have added 'Version.CURRENT'
…shots Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com>
Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com>
002abda
to
8e7af89
Compare
❌ Gradle check result for 8e7af89: 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: Lakshya Taragi <lakshya.taragi@gmail.com>
8e7af89
to
cefa0bc
Compare
❕ Gradle check result for cefa0bc: 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. |
@@ -115,6 +115,8 @@ public class InternalClusterInfoService implements ClusterInfoService, ClusterSt | |||
private volatile Map<String, FileCacheStats> nodeFileCacheStats; | |||
private volatile IndicesStatsSummary indicesStatsSummary; | |||
// null if this node is not currently the cluster-manager | |||
|
|||
private volatile long primaryStoreSize; |
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.
Looks like we already have final Map<String, Long> shardSizes;
, which store shard level sizes. Can we just use this to retrieve shard sizes for v2 ?
This PR is stalled because it has been open for 30 days with no activity. |
Description
primaryStoreSize
is calculated at the time of consolidation of shard level info inInternalClusterInfoService.java
primaryStoreSize
is further included inClusterInfo
. It also implementsgetPrimaryStoreSize
, which can be called during the snapshot creation to persist this piece of information as the snapshot's size in bytes insideSnapshotInfo
SnapshotInfo
and returned for shallow V2 snapshotsassertBusy
to prevent flakiness inorg.opensearch.snapshots.SnapshotStatusApisIT.testStatusAPICallForShallowCopySnapshot
Related Issues
Resolves this comment of #15463
Check List
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.