RHINENG-30670: remove multi IN-list condition - #2336
Conversation
PostgreSQL 17 introduced optimizer changes, including how IN-lists are processed with btree indexes. This seems to cause OOMkill during BIND phase (query planning/parameter binding), a single connection's memory consumption increases to approximately 2.6 GB.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates bulk NEVRA package-cache reads to perform individual equality-based lookups instead of constructing a multi-column IN-list, mitigating PostgreSQL 17 OOM risk during query planning while preserving found and missing package tracking. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="evaluator/package_cache.go" line_range="363-375" />
<code_context>
for _, nevraString := range nevraStrings {
</code_context>
<issue_to_address>
**issue (performance):** ReadByNevras now performs one database transaction and query for every valid NEVRA instead of one batch query, so a request containing a large list executes N sequential queries and incurs N query-planning/round-trip costs.
**Triggers:** When callers pass a large number of cache misses in one request.
**Suggested fix:** Use bounded batching or a temporary/table-valued input strategy that avoids PostgreSQL's pathological multi-column IN-list planning without issuing one transaction per NEVRA.
</issue_to_address>
### Comment 2
<location path="evaluator/package_cache.go" line_range="363-375" />
<code_context>
for _, nevraString := range nevraStrings {
</code_context>
<issue_to_address>
**issue (bug_risk):** ReadByNevras appends a result for every input occurrence, so duplicate valid NEVRA strings produce duplicate PackageCacheMetadata entries; the previous implementation deduplicated inputs through the parsed map before querying and returned each database package once.
**Triggers:** When the input slice contains duplicate valid NEVRA strings.
**Suggested fix:** Deduplicate parsed NEVRAs before querying, or track returned package identities and append each package only once.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: evaluator/package_cache.go:375, evaluator/package_cache.go:375
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2336 +/- ##
==========================================
- Coverage 59.06% 58.99% -0.08%
==========================================
Files 150 150
Lines 9615 9603 -12
==========================================
- Hits 5679 5665 -14
- Misses 3342 3346 +4
+ Partials 594 592 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PostgreSQL 17 introduced optimizer changes, including how IN-lists are processed with btree indexes. This seems to cause OOMkill during BIND phase (query planning/parameter binding), a single connection's memory consumption increases to approximately 2.6 GB.
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Replace multi-NEVRA database queries with individual package lookups to avoid PostgreSQL 17 optimizer memory issues.
Bug Fixes:
Enhancements: