search

opensearch-project / OpenSearch

🔎 Open source distributed and RESTful search engine.
https://opensearch.org/docs/latest/opensearch/index/
Apache License 2.0
9.84k stars 1.83k forks source link

[Bugfix] Fix cache maximum size settings not working properly with pluggable caching #16636

Open peteralfonsi opened 1 week ago

peteralfonsi commented 1 week ago

Description

Fixes a bug (https://github.com/opensearch-project/OpenSearch/issues/16631) where max size settings for cache implementations were not working correctly when pluggable caching is on.

The cache implementations like OpenSearchOnHeapCache had been changed so that the max size value from the config overrode the value from their setting. This allowed the TieredSpilloverCache to send different sizes into the cache config when constructing its segments. However, the IndicesRequestCache was also putting its default 1% of heap size value into the cache config, and this overrode the setting values even when pluggable caching was on.

Fixes this by having the IRC only put a max size value into the config if pluggable caching is off.

Adds UTs around this. Also tested manually with different combinations of settings.

Related Issues

Resolves https://github.com/opensearch-project/OpenSearch/issues/16631

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.

github-actions[bot] commented 1 week ago

:white_check_mark: Gradle check result for 3d1fe7350b0fb8bbb293e036d704ed95d4942dd6: SUCCESS

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.08%. Comparing base (10873f1) to head (3d1fe73). Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...va/org/opensearch/indices/IndicesRequestCache.java 86.95% 3 Missing :warning:
.../opensearch/common/cache/service/CacheService.java 80.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #16636 +/- ## ============================================ - Coverage 72.15% 72.08% -0.08% - Complexity 65145 65162 +17 ============================================ Files 5315 5318 +3 Lines 303573 303833 +260 Branches 43925 43962 +37 ============================================ - Hits 219039 219003 -36 - Misses 66587 66925 +338 + Partials 17947 17905 -42 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 week ago

:x: Gradle check result for 469006a9e6fb2de0093e10e93c2981d548e7d930: 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?

github-actions[bot] commented 1 week ago

:x: Gradle check result for 76ad2ef113c5f1dcffb81817541b0e349378a638: 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?

peteralfonsi commented 1 week ago

Flaky tests: https://github.com/opensearch-project/OpenSearch/issues/14568, https://github.com/opensearch-project/OpenSearch/issues/16015

github-actions[bot] commented 3 days ago

:x: Gradle check result for c75fb9fdeb644a4f8a2bbbef4136d026691bbb7d:

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?