[LFXV2-924] Optimize OpenSearch queries with filter context for 30% performance improvement #27
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe OpenSearch bool query in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)internal/infrastructure/opensearch/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*_test.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR optimizes OpenSearch query performance by converting exact-match term queries from query context (must clauses) to filter context (filter clauses), achieving a 30% performance improvement and 4x better consistency based on benchmark results with 10M documents.
Key Changes:
- Changed the query template from using
musttofilterfor all queries in the main boolean clause - Leverages OpenSearch's filter context caching and elimination of scoring overhead for exact-match queries
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/infrastructure/opensearch/template.go (1)
12-66: multi_match query for.Nameshould not be moved to filter context without explicit intent.The code moves the
multi_matchquery (lines 44-55) to filter context, but the PR description lists only exact-match term queries forlatest,public,object_type,parent_refs, andtags— it does not mention thenamefield.This is a behavioral change: multi_match queries in filter context do not contribute to relevance scoring, so name matches will no longer affect result ranking. All matching results are treated equally (binary: match or no-match).
The tests use Name criteria but with mocked responses (all with Score: 1.0) and do not validate result ordering for name-based searches. The query template does specify explicit sort (lines 90-97), which can override _score, but the impact of removing scoring should be verified.
Required action: Clarify whether moving multi_match to filter is intentional, update the PR description accordingly, and verify result ordering for name-based searches is unaffected (either by explicit sort or confirmed no-op).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/infrastructure/opensearch/template.go
🧰 Additional context used
📓 Path-based instructions (1)
internal/infrastructure/opensearch/**
📄 CodeRabbit inference engine (CLAUDE.md)
Put OpenSearch implementations for resource search under internal/infrastructure/opensearch/
Files:
internal/infrastructure/opensearch/template.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
- GitHub Check: MegaLinter
…mprovement Changed from query context (must clauses) to filter context for exact-match term queries. Benchmark results on 10M documents show: - Query context (must): 136ms avg, 68ms variance - Filter context: 94ms avg, 16ms variance - 30% performance improvement with 4x better consistency Filter context provides: - No score calculation overhead - Better query caching - Lower CPU usage - More predictable latency All exact-match queries (latest, public, object_type, parent_refs, tags) now use filter clauses instead of must clauses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <[email protected]>
andrest50
force-pushed
the
andrest50/query-filter
branch
from
cc47049 to
85b5aa1
Compare
| "query": { | ||
| "bool": { | ||
| "must": [ | ||
| "filter": [ |
There was a problem hiding this comment.
Thank you for collecting all the performance results. Nice work!
Based on the tradeoffs, maybe we should include a conditional to use must for the name query parameter (I think we still need some querying using relevance/scoring, which I understand must is built for) and use filter for the others (exact match). For example:
{
"size": 50,
"query": {
"bool": {
"must": [
{
"multi_match": {
"query": "linux fouDNation",
"type": "bool_prefix",
"fields": [
"name_and_aliases",
"name_and_aliases._2gram",
"name_and_aliases._3gram"
]
}
}
],
"filter": [
{
"term": {
"latest": true
}
},
{
"term": {
"object_type": "project"
}
},
{
"term": {
"data.category": "Sandbox"
}
},
{
"term": {
"parent_refs": "project:16b22a7a-0992-4f4a-a825-534669bde81d"
}
}
]
}
},
"sort": [
...
]
}
There was a problem hiding this comment.
This is good. Thank you! Eric had told me that using the filter clause doesn't really make sense for the search - you might be right that some terms could use filter and others must, although for now I'm going to close the PR.
|
We don't want to switch to using the |
Ticket
LFXV2-924
Summary
Optimized OpenSearch query performance by switching from query context (
mustclauses) to filter context for exact-match term queries. This change provides a 30% performance improvement with 4x better consistency at scale.Performance Results (10M documents, 50 iterations)
Why Filter Context Is Better
Changes
Changed all exact-match term queries in the OpenSearch query template from
mustclauses tofilterclauses:latestfieldpublicfieldobject_typefieldparent_refsfieldtagsfield (inTagsAll)The
shouldclauses for optional tag matching remain in query context as intended.Test Plan
🤖 Generated with Claude Code