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

[Remote Store] Add Segment download stats to remotestore stats API #8718

Merged

Conversation

shourya035
Copy link
Member

@shourya035 shourya035 commented Jul 17, 2023

Re-opening since #8440 was closed accidentally

Description

  • Adding segment download stats to the _remotestore/stats API
  • Refactored API output to accommodate both segment and translog stats

The new API output would show a blank JSON node {} if the corresponding values in that node are zero. This has been done intentionally to ensure that we do not show 0 stats to the end user which in turn would avoid confusion. This is important since a newly created replica would not upload anything to the remote store and a newly created primary would not download from the remote store.

The only exception to this behavior would be a primary shard which has been restored from a remote store. In that case, this API would show both segment upload and download stats for the shard. This behavior would be documented as well.

Sample API outputs from running in local dev setup

P.S: RemoteRefreshSegmentTracker was renamed to RemoteRefreshTransferTracker, because of which the number of Files changes has increased from 13 to 17.

Related Issues

Resolves #8395

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya Dutta Biswas <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled
      1 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards

@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #8718 (91378d1) into main (b2a7348) will decrease coverage by 0.17%.
The diff coverage is 90.08%.

❗ Current head 91378d1 differs from pull request most recent head 508eb22. Consider uploading reports for the commit 508eb22 to get more accurate results

@@             Coverage Diff              @@
##               main    #8718      +/-   ##
============================================
- Coverage     71.04%   70.88%   -0.17%     
+ Complexity    57246    57071     -175     
============================================
  Files          4764     4757       -7     
  Lines        270153   270016     -137     
  Branches      39532    39490      -42     
============================================
- Hits         191937   191399     -538     
- Misses        62027    62433     +406     
+ Partials      16189    16184       -5     
Files Changed Coverage Δ
...testore/stats/TransportRemoteStoreStatsAction.java 44.44% <0.00%> (-1.51%) ⬇️
...search/index/shard/RemoteStoreRefreshListener.java 84.06% <ø> (-0.55%) ⬇️
...ex/remote/RemoteRefreshSegmentPressureService.java 78.75% <63.63%> (ø)
...in/java/org/opensearch/index/shard/IndexShard.java 69.66% <80.85%> (+0.62%) ⬆️
...er/remotestore/stats/RemoteStoreStatsResponse.java 74.28% <84.61%> (+27.22%) ⬆️
...in/cluster/remotestore/stats/RemoteStoreStats.java 94.25% <94.80%> (-0.99%) ⬇️
...rch/index/remote/RemoteSegmentTransferTracker.java 91.87% <100.00%> (ø)

... and 566 files with indirect coverage changes

@shourya035 shourya035 marked this pull request as ready for review July 17, 2023 10:33
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Shourya Dutta Biswas <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

Gradle Check (Jenkins) Run Completed with:

@sachinpkale
Copy link
Member

This is a user facing change. We should have a corresponding changlelog entry.

@sachinpkale
Copy link
Member

Going ahead without changelog entry. Please make sure to add it in the next PR.

@sachinpkale sachinpkale changed the title [Remote Store] Adding Segment download stats to remotestore stats API [Remote Store] Add Segment download stats to remotestore stats API Aug 1, 2023
@sachinpkale sachinpkale merged commit 77074b4 into opensearch-project:main Aug 1, 2023
11 of 12 checks passed
@sachinpkale sachinpkale added the backport 2.x Backport to 2.x branch label Aug 1, 2023
@opensearch-trigger-bot
Copy link
Contributor

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:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-8718-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 77074b4a81f88bab44e9e39e0b6eb9323880e740
# Push it to GitHub
git push --set-upstream origin backport/backport-8718-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

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

@shourya035 shourya035 deleted the segment-download-stats-2 branch August 1, 2023 12:39
@ashking94
Copy link
Member

@shourya035 let's raise a manual backport for this.

private final Logger deletesLogger;

public final DirectoryFileTransferTracker directoryFileTransferTracker;
Copy link
Member

@mch2 mch2 Aug 1, 2023

Choose a reason for hiding this comment

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

This is now going to apply to every use of the local store.copyFrom. I'm not sure this belongs here? Can we wrap and collect these required stats directly in the remote store directory or a related component?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should think about making this tracking optional by way of an expert setting?

@dreamer-89
Copy link
Member

@shourya035 : Can you please create the backport PR to 2.x for this change. This is blocking other PRs which have changes on top of yours.

shourya035 added a commit to shourya035/OpenSearch that referenced this pull request Aug 3, 2023
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
(cherry picked from commit 77074b4)
shourya035 added a commit to shourya035/OpenSearch that referenced this pull request Aug 3, 2023
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
(cherry picked from commit 77074b4)
shourya035 added a commit to shourya035/OpenSearch that referenced this pull request Aug 3, 2023
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
(cherry picked from commit 77074b4)
sachinpkale pushed a commit that referenced this pull request Aug 3, 2023
…8718) (#9089)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
(cherry picked from commit 77074b4)
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
Signed-off-by: Kaushal Kumar <[email protected]>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
Signed-off-by: Ivan Brusic <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…pensearch-project#8718)

---------

Signed-off-by: Shourya Dutta Biswas <[email protected]>
Signed-off-by: Shourya <[email protected]>
Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
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 skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Remote Store] RFC - Adding segment download metrics to remotestore stats API
6 participants