-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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] [Bugfix] [Tiered Caching] Fixes issues when integrating tiered cache with disk cache #13801
[Backport 2.x] [Bugfix] [Tiered Caching] Fixes issues when integrating tiered cache with disk cache #13801
Conversation
…with disk cache (opensearch-project#13784) --------- Signed-off-by: Peter Alfonsi <[email protected]> Co-authored-by: Peter Alfonsi <[email protected]> (cherry picked from commit e67ced7) Signed-off-by: Peter Alfonsi <[email protected]>
❌ Gradle check result for ad77481: 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? |
@msfroh @dblock @VachaShah @sohami Would anyone be able to merge this backport PR? |
We do need a passing build. When you see a flaky test, force push an update to re-trigger CI. I did click the button in the meantime, but you can self-serve faster ;) |
❌ Gradle check result for ad77481: 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 2aed568: null 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]>
@dblock I think I finally have a green build with DCO, would you be able to reapprove and merge? |
Signed-off-by: Peter Alfonsi <[email protected]>
❌ Gradle check result for 125496b: 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]>
❌ Gradle check result for 7ea9533: 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]>
❌ Gradle check result for 103f0eb: 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]>
…g tiered cache with disk cache (opensearch-project#13801) * [Bugfix] [Tiered Caching] Fixes issues when integrating tiered cache with disk cache (opensearch-project#13784) --------- Signed-off-by: Peter Alfonsi <[email protected]> Co-authored-by: Peter Alfonsi <[email protected]> (cherry picked from commit e67ced7) Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun dco approval :( Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * spotlessApply after merge conflict Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> * rerun gradle Signed-off-by: Peter Alfonsi <[email protected]> --------- Signed-off-by: Peter Alfonsi <[email protected]> Signed-off-by: Peter Alfonsi <[email protected]> Co-authored-by: Peter Alfonsi <[email protected]> Signed-off-by: kkewwei <[email protected]>
Original PR: #13784
Description
Fixes issues that prevented us from using the tiered spillover cache with EhcacheDiskCache as the disk cache:
These issues weren't caught in automated testing since the TSC lives in a module and the disk cache lives in a separate plugin, so we couldn't IT them together and the issues were only found during manual testing. Confirmed that after these fixes are in place, we can use the feature as desired using:
./gradlew run -Dtests.opensearch.opensearch.experimental.feature.pluggable.caching.enabled=true -Dtests.opensearch.indices.requests.cache.store.name=tiered_spillover -Dtests.opensearch.indices.requests.cache.tiered_spillover.onheap.store.name=opensearch_onheap -Dtests.opensearch.indices.requests.cache.tiered_spillover.disk.store.name=ehcache_disk -Dtests.opensearch.indices.requests.cache.tiered_spillover.disk.store.policies.took_time.threshold=0ms -PinstalledPlugins="['cache-ehcache']" -Dtests.opensearch.indices.requests.cache.ehcache_disk.storage.path="/Volumes/workplace/opensearch/OpenSearch/build/testclusters/runTask-0/data/nodes/0/indices/request_cache"
I've bundled these bugfixes together, as we can't really confirm any one of the fixes works until the manual test runs correctly, which requires all of the fixes.
Related Issues
Part of tiered caching
Check List
- [N/A] API changes companion pull request created.[N/A]
Commit changes are listed out in CHANGELOG.md file (See: Changelog)[N/A]
Public documentation issue/PR createdBy 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.