perf: api/v3/license query performance#2497
Queued
rh-jfuller wants to merge 1 commit into
Queued
Conversation
Contributor
Reviewer's GuideThis PR adds a dedicated index on sbom_license_expanded.license_id via a new migration to dramatically improve the NOT EXISTS anti-join used by GET /v3/license, reducing query time from ~277s to ~11ms while keeping the schema otherwise unchanged. Sequence diagram for GET v3 license using idx_sle_license_idsequenceDiagram
actor Client
participant ApiService as Api_v3_license
participant Postgres
Client->>ApiService: GET /v3/license
ApiService->>Postgres: SELECT DISTINCT license.text
Postgres-->>ApiService: NestedLoopAntiJoin
ApiService->>Postgres: Index Only Scan using idx_sle_license_id
Postgres-->>ApiService: rows with NOT EXISTS sbom_license_expanded
ApiService-->>Client: license texts (fast response)
Entity relationship diagram for sbom_license_expanded license_id indexerDiagram
license {
int id pk
string text
}
sbom_license_expanded {
int sbom_id pk
int license_id pk
string idx_sle_license_id "index on license_id"
}
license ||--o{ sbom_license_expanded : "license_id"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider using the SeaORM migration DSL (e.g.,
manager.create_index(...)) instead ofexecute_unpreparedso the index definition stays type-safe and consistent with other migrations. - The index name
idx_sle_license_iddiffers from thetmp_idx_sle_license_idshown in the explain plan in the description; aligning these names (or updating the description) will help avoid confusion when debugging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using the SeaORM migration DSL (e.g., `manager.create_index(...)`) instead of `execute_unprepared` so the index definition stays type-safe and consistent with other migrations.
- The index name `idx_sle_license_id` differs from the `tmp_idx_sle_license_id` shown in the explain plan in the description; aligning these names (or updating the description) will help avoid confusion when debugging.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ctron
approved these changes
Jul 14, 2026
b307cc6 to
884f252
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2497 +/- ##
==========================================
+ Coverage 71.66% 71.68% +0.01%
==========================================
Files 453 454 +1
Lines 27517 27525 +8
Branches 27517 27525 +8
==========================================
+ Hits 19719 19730 +11
+ Misses 6667 6657 -10
- Partials 1131 1138 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
884f252 to
6538864
Compare
Contributor
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider using the schema builder (
Index::create/Index::drop) instead ofexecute_unpreparedso the migration stays consistent with the rest of the migration code and benefits from compile-time structure and database abstraction. - Using
CREATE INDEX IF NOT EXISTSandDROP INDEX IF EXISTSin a migration can hide unexpected schema drift; if this index is required for performance, you may want the migration to fail loudly instead of silently skipping when the index state is different than expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using the schema builder (`Index::create` / `Index::drop`) instead of `execute_unprepared` so the migration stays consistent with the rest of the migration code and benefits from compile-time structure and database abstraction.
- Using `CREATE INDEX IF NOT EXISTS` and `DROP INDEX IF EXISTS` in a migration can hide unexpected schema drift; if this index is required for performance, you may want the migration to fail loudly instead of silently skipping when the index state is different than expected.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or 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
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.
The query in api/v3/license is GET /v3/license (license/service/mod.rs:318-328) eg. the NOT EXISTS subquery in:
is very slow and ties up postgres resources.
The sbom_license_expanded table has a composite primary key (sbom_id, license_id).
In a B-tree composite index, the leading column is
sbom_idwhich a correlated lookup onlicense_idalone cannot use this index.Postgres is falling back to a sequential scan of the entire
sbom_license_expanded tablefor every row in license. That is an O(N*M) nested loop with no index support, explaining the 277-second execution time!The fix is to add index:
Before
After
going from 277 seconds to 10.8 ms is not too shabby ;)
Summary by Sourcery
Enhancements: