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

[Feature Request] Inner queries for has_parent and has_child should be eligible for caching in the query cache #17096

Open
jhinch-at-atlassian-com opened this issue Jan 23, 2025 · 3 comments
Labels
enhancement Enhancement or improvement to existing feature or request Search:Performance

Comments

@jhinch-at-atlassian-com

Is your feature request related to a problem? Please describe

GlobalOrdinalsQuery which is used by has_parent and has_child queries is not eligible for query caching as it makes use of global ordinals and so isn't segment cacheable. However its inner query should be as it doesn't use the global ordinals but currently isn't due to HasChildQueryBuilder.LateParsingQuery not propagating the query cache to the IndexerSearcher used for the inner query.

Describe the solution you'd like

Allow for the inner query to be cached, improving performance of join queries

Related component

Search:Performance

Describe alternatives you've considered

No response

Additional context

No response

@jhinch-at-atlassian-com jhinch-at-atlassian-com added enhancement Enhancement or improvement to existing feature or request untriaged labels Jan 23, 2025
@kkewwei
Copy link
Contributor

kkewwei commented Jan 23, 2025

It makes sense to me. As long as the query frequency is high, inner query to be cached.

@sandeshkr419
Copy link
Contributor

@kkewwei @msfroh - Do you guys have any findings here?

@msfroh
Copy link
Collaborator

msfroh commented Jan 29, 2025

I agree with @kkewwei that it makes sense to cache them.

I see this line that explicitly disables the query cache on the newly-created IndexSearcher, which looks intentional, though:

I don't really understand why that was done and git blame shows that line coming from a merge commit, so it will take some more digging to understand the reasoning.

@jhinch-at-atlassian-com -- it sounds like you have an idea of how/where to propagate the cache. Do you want to give this issue a shot?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Search:Performance
Projects
Status: 🆕 New
Development

No branches or pull requests

4 participants