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

[Backport 2.x] [Tiered Caching] Expose new cache stats API (#13237) #13456

Merged

Conversation

peteralfonsi
Copy link
Contributor

Original PR: #13237

Note: I have changed V_3_0_0 to V_2_14_0 in the relevant places for this backport, and added TODOs to change this value to V_2_14_0 in main in the future.

Description

This change exposes a new cache stats API described in this issue. Currently the only supported <cache_type> parameter is request_cache, but more can be added easily. This API supports stats aggregation by arbitrary dimensions, which are stored in the keys (ICacheKey objects) that are passed to ICache implementations. In the IRC, these dimensions are "indices", "shards", and "tier" (if a TieredSpilloverCache is used as the cache implementation, see #13236).

Tested manually and with an IT to check aggregation by various levels in the IRC.

No public doc PR for now, as the tiered caching feature is not yet complete and we are waiting to add doc PRs until it's fully ready for use.

Related Issues

Part of #12258

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)
    - [N/A] 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.

Step 3 out of 4

---------

Signed-off-by: Peter Alfonsi <[email protected]>
Co-authored-by: Peter Alfonsi <[email protected]>
(cherry picked from commit 2a37bdd)
Peter Alfonsi added 2 commits April 29, 2024 15:23
@peteralfonsi peteralfonsi force-pushed the backport/backport-13237-to-2.x branch from bc553c7 to 474c19f Compare April 29, 2024 23:01
Copy link
Contributor

❌ Gradle check result for bc553c7: 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
Contributor

✅ Gradle check result for 929635c: SUCCESS

Copy link

codecov bot commented Apr 29, 2024

Codecov Report

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

Project coverage is 71.08%. Comparing base (0dd892c) to head (1f24c30).
Report is 209 commits behind head on 2.x.

Files Patch % Lines
...est/action/admin/cluster/RestNodesStatsAction.java 0.00% 15 Missing and 1 partial ⚠️
...pensearch/common/cache/service/NodeCacheStats.java 48.00% 10 Missing and 3 partials ⚠️
...h/action/admin/indices/stats/CommonStatsFlags.java 63.15% 5 Missing and 2 partials ⚠️
.../common/cache/stats/ImmutableCacheStatsHolder.java 94.77% 3 Missing and 4 partials ⚠️
...in/java/org/opensearch/common/cache/CacheType.java 57.14% 6 Missing ⚠️
...rch/action/admin/cluster/node/stats/NodeStats.java 44.44% 3 Missing and 2 partials ⚠️
.../opensearch/common/cache/service/CacheService.java 0.00% 5 Missing ⚠️
.../opensearch/core/common/io/stream/StreamInput.java 63.63% 2 Missing and 2 partials ⚠️
...src/main/java/org/opensearch/node/NodeService.java 33.33% 0 Missing and 2 partials ⚠️
.../opensearch/cache/store/disk/EhcacheDiskCache.java 83.33% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #13456      +/-   ##
============================================
- Coverage     71.28%   71.08%   -0.20%     
- Complexity    60145    60880     +735     
============================================
  Files          4957     5024      +67     
  Lines        282799   287007    +4208     
  Branches      41409    41936     +527     
============================================
+ Hits         201591   204019    +2428     
- Misses        64189    65731    +1542     
- Partials      17019    17257     +238     

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

Copy link
Contributor

❌ Gradle check result for 3030b81: 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
Contributor

❌ Gradle check result for 474c19f: 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: Peter Alfonsi <[email protected]>
Copy link
Contributor

❌ Gradle check result for 1e04ce5: 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: Peter Alfonsi <[email protected]>
Copy link
Contributor

❕ Gradle check result for 1f24c30: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction
      1 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=indices.get_field_mapping/20_missing_field/Return empty object if field doesn't exist, but index does}

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

@msfroh msfroh merged commit 67001e2 into opensearch-project:2.x Apr 30, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants