Move TSC took-time policy to guard both heap and disk tier #17190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
peteralfonsi
requested review from
anasalkouz,
andrross,
ashking94,
Bukhtawar,
CEHENKLE,
cwperks,
dblock,
dbwiddis,
gbbafna,
jainankitk,
kotwanikunal,
linuxpi,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
github-actions
bot
added
enhancement
peteralfonsi
added
backport
jainankitk
approved these changes
Feb 19, 2025
...ache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheSettings.java
Show resolved
Hide resolved
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
|
❌ Gradle check result for d74ae2d: 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 d74ae2d: 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? |
|
Flaky test: #17154 |
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
sgup432
reviewed
Feb 24, 2025
modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for aee2d8e: 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>
|
❌ Gradle check result for 9104303: 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? |
|
Flaky test: #14559 |
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
|
❌ Gradle check result for 128506d: 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>
peteralfonsi
force-pushed
the
tsc-policy-change
branch
from
128506d to
cb1bbf4
Compare
|
❌ Gradle check result for cb1bbf4: 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>
sgup432
approved these changes
Feb 27, 2025
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Feb 27, 2025
* Move TSC took-time policy to guard both heap and disk tier Signed-off-by: Peter Alfonsi <petealft@amazon.com> * changelog Signed-off-by: Peter Alfonsi <petealft@amazon.com> * spotless apply Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Addressed Sagar's comment Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Add missing javadoc Signed-off-by: Peter Alfonsi <petealft@amazon.com> * address round 2 of comments Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Add removal notification to put() Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Fix incorrect stats hit when cache entry rejected by policy Signed-off-by: Peter Alfonsi <petealft@amazon.com> * rerun gradle Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Fixed more broken stats Signed-off-by: Peter Alfonsi <petealft@amazon.com> * rerun gradle Signed-off-by: Peter Alfonsi <petealft@amazon.com> * Addressed more comments Signed-off-by: Peter Alfonsi <petealft@amazon.com> * make policy rejections count as neither hit or miss Signed-off-by: Peter Alfonsi <petealft@amazon.com> * rerun gradle Signed-off-by: Peter Alfonsi <petealft@amazon.com> * remove potential double-loading Signed-off-by: Peter Alfonsi <petealft@amazon.com> * rerun gradle Signed-off-by: Peter Alfonsi <petealft@amazon.com> * remove removalNotification Signed-off-by: Peter Alfonsi <petealft@amazon.com> * rerun gradle Signed-off-by: Peter Alfonsi <petealft@amazon.com> --------- Signed-off-by: Peter Alfonsi <petealft@amazon.com> Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com> Co-authored-by: Peter Alfonsi <petealft@amazon.com> (cherry picked from commit b1e66b3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
jainankitk
pushed a commit
that referenced
this pull request
Feb 28, 2025
…17478) * Move TSC took-time policy to guard both heap and disk tier * changelog * spotless apply * Addressed Sagar's comment * Add missing javadoc * address round 2 of comments * Add removal notification to put() * Fix incorrect stats hit when cache entry rejected by policy * rerun gradle * Fixed more broken stats * rerun gradle * Addressed more comments * make policy rejections count as neither hit or miss * rerun gradle * remove potential double-loading * rerun gradle * remove removalNotification * rerun gradle --------- (cherry picked from commit b1e66b3) Signed-off-by: Peter Alfonsi <petealft@amazon.com> Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Peter Alfonsi <petealft@amazon.com>
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Changes the TieredSpilloverCache's minimum took-time policy so that queries must take >10 ms to enter either tier, instead of just to enter the disk tier. This is desirable because when we allow caching size > 0 queries in the request cache, the number of cacheable queries may go way up, and we should avoid flooding the heap tier with overly cheap queries.
Note the setting key
tiered_spillover.disk.store.policies.took_time.thresholdis unchanged, for backwards compatibility.Related Issues
Resolves #16162
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.