-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Disable caching when queries are profiled #48195
Conversation
This change disables the query and request cache when profile is set to true in the request. This means that profiled queries will not check caches to execute the query and the result will never be added in the cache either. Closes elastic#33298
Pinging @elastic/es-search (:Search/Search) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question.
if (profiler != null) { | ||
// disable query caching on profiled query | ||
setQueryCachingPolicy(NEVER_CACHE_POLICY); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity, why isn't returning false on ProfileWeight enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the sub-query might be cacheable so returning false
only ensures that we don't cache the query at the profile weight level. Sub queries are still eligible for caching so we have to disable caching entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we wrapped sub weights too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do and that works for intermediate level but leaves are still eligible for caching so a simple range query could get cached ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't the range weight be wrapped as well?
I looked a bit more into this and maybe we could simplify by creating the weight manually instead of using IndexSearcher, i.e. replacing the call to |
This change disables the query and request cache when profile is set to true in the request. This means that profiled queries will not check caches to execute the query and the result will never be added in the cache either. Closes #33298
This change disables the query and request cache when
profile is set to true in the request. This means that profiled queries
will not check caches to execute the query and the result will never be
added in the cache either.
Closes #33298