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] Cache tier policies (#12542) #12696

Merged

Conversation

peteralfonsi
Copy link
Contributor

Signed-off-by: Peter Alfonsi petealft@amazon.com
Co-authored-by: Peter Alfonsi petealft@amazon.com
(cherry picked from commit ccdf3ff)

Description

Original PR: #12542

This PR adds caching tier policies. In a tiered caching setup, or even for a single-tier cache, we might not want to allow all values into all tiers, so we want to define policies to allow or reject values. For example, we might not want to add values that took a very short time to compute to a disk tier, since the time to recompute them could be shorter than the time to get them back from the disk.

This PR adds an interface for policies and integrates them to restrict access to the disk tier in the TieredSpilloverCache. Currently the only non-test implementation is the took time policy, but it should be easy to add more as they're needed.

When QuerySearchResult values are written into the IndicesRequestCache, we first write a CachePolicyInfoWrapper object, which has any values we might want to provide to policies. This way policies don't have to read the entire QSR (or other value type V). This wrapper is discarded when values come out of the cache.

CacheConfig<K, V> is modified to accept a generic Function<V, CachePolicyInfoWrapper>, which is used to get such a wrapper from cache values. It can be passed along to policies the cache will use.

Related Issues

This is part of the tiered caching feature (#10024).

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.

* Adds policy interface and took time policy impl

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Changes IndicesService to write a CachePolicyInfoWrapper before the QSR

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Moved took time logic from QSR to IndicesService

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* spotlessApply

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed ansjcy's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Partial rebase on most recent changes

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Integrated policies with new TSC changes

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Reverted unintended change to idea/vcs.xml

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* javadocs

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* github actions

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Set default threshold value to 10 ms

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed Sorabh's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed Sorabh's second round of comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Set cachedQueryParser in IRC

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed Sorabh's comments besides dynamic setting

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Removed dynamic setting, misc comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Added changelog entry

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Added missing javadoc

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fixed failed gradle run

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Added setting validation test

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle for flaky IT

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* javadocs

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
(cherry picked from commit ccdf3ff)
@peteralfonsi peteralfonsi changed the title [Tiered Caching] (Backport 2.x) Cache tier policies (#12542) [Backport 2.x] [Tiered Caching] Cache tier policies (#12542) Mar 15, 2024
Copy link
Contributor

github-actions bot commented Mar 15, 2024

Compatibility status:

Checks if related components are compatible with change 431293b

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/performance-analyzer.git]

Copy link
Contributor

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

@sohami
Copy link
Collaborator

sohami commented Mar 15, 2024

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

Related to #11933

Copy link
Contributor

❌ Gradle check result for 43211a6: ABORTED

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 <petealft@amazon.com>
Copy link
Contributor

❌ Gradle check result for 8176e07: 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 <petealft@amazon.com>
Copy link
Contributor

❕ Gradle check result for 8176e07: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.remotestore.multipart.RemoteStoreMultipartIT.testOverriddenBufferInterval
      1 org.opensearch.indices.replication.SegmentReplicationAllocationIT.testAllocationWithDisruption
      1 org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction

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 Mar 16, 2024

Codecov Report

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

Project coverage is 71.20%. Comparing base (0dd892c) to head (431293b).
Report is 34 commits behind head on 2.x.

Files Patch % Lines
.../cache/common/tier/TieredSpilloverCachePlugin.java 0.00% 3 Missing ⚠️
...va/org/opensearch/indices/IndicesRequestCache.java 25.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #12696      +/-   ##
============================================
- Coverage     71.28%   71.20%   -0.08%     
+ Complexity    60145    60126      -19     
============================================
  Files          4957     4961       +4     
  Lines        282799   283104     +305     
  Branches      41409    41444      +35     
============================================
- Hits         201591   201590       -1     
- Misses        64189    64439     +250     
- Partials      17019    17075      +56     

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

Copy link
Contributor

❕ Gradle check result for f001e49: UNSTABLE

  • TEST FAILURES:
      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.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Copy link
Contributor

❌ Gradle check result for 7fe7fb2: 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 <petealft@amazon.com>
Copy link
Contributor

❌ Gradle check result for 0c5b9fb: 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 <petealft@amazon.com>
Copy link
Contributor

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

@sohami
Copy link
Collaborator

sohami commented Mar 17, 2024

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

#5425
#11933
#10006

Copy link
Contributor

❕ Gradle check result for 431293b: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testStrictWeightedRoutingWithCustomString_FailOpenEnabled

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

@peteralfonsi
Copy link
Contributor Author

#12708

@sohami sohami merged commit d3c51a4 into opensearch-project:2.x Mar 18, 2024
29 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