Repository navigation
perf(db): index ingestion and shared-frame lookups - #705
Merged
Merged
Conversation
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| revision: str = "e7a4b9c3d5f2" |
| from alembic import op | ||
|
|
||
| revision: str = "e7a4b9c3d5f2" | ||
| down_revision: Union[str, None] = "d6f3a8b2c4e1" |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #705 +/- ##
=======================================
Coverage 93.87% 93.88%
=======================================
Files 59 59
Lines 3218 3221 +3
=======================================
+ Hits 3021 3024 +3
Misses 197 197
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:
|
Member
|
Thanks for the PR ! Maybe could you check #664 first and share ideas? |
|
|
||
| revision: str = "e7a4b9c3d5f2" | ||
| down_revision: Union[str, None] = "d6f3a8b2c4e1" | ||
| branch_labels: Union[str, Sequence[str], None] = None |
| revision: str = "e7a4b9c3d5f2" | ||
| down_revision: Union[str, None] = "d6f3a8b2c4e1" | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None |
Member
Author
I forgot to reply, but I went through it, and integrated the additional benefits from that PR here so that we have a net positive PR, with minimal edits, and major performance boosts 🤓 |
MateoLostanlen
approved these changes
Oct 8, 2026
MateoLostanlen
left a comment
Member
There was a problem hiding this comment.
Thanks @frgfm a lot for this PR, great to get these indexes in!
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.
This PR includes the useful indexes from #664. It adds a separate index for unassigned detections: detections that are not yet grouped into a sequence.
The main gain over #664 is narrower than the gain over the baseline. For the unassigned-detection query, this PR took 1.83 ms instead of 7.97 ms with a generic query plan. That is 77% less time, or 4.4x faster. The shared-frame and other common indexes come from #664. This benchmark does not show an extra gain for those shared paths.
An index works like a sorted list. #664 finds recent unassigned rows across all cameras, then checks the camera and pose (viewing direction). #705 can go directly to the rows for one camera and pose.
flowchart TB Q["Find recent unassigned detections<br/>for one camera and pose"] Q --> A["#664: find recent unassigned rows<br/>across all cameras"] A --> B["Check camera and pose<br/>Remove unrelated rows"] B --> R["Return the matching detections"] Q --> C["#705: use the extra index<br/>Go to this camera, pose, and time"] C --> RThe saved query plans show the smaller search: #664 found 6,000 rows, then kept 120 after checking the camera and pose. #705 found those 120 rows directly. Both queries returned the same detections.
A generic plan is one query plan reused for different input values. We tested that mode and the default setting, which lets PostgreSQL choose its plan.
The extra index uses
(camera_id, pose_id, created_at)and includes only rows wheresequence_id IS NULL. It occupies about 16 MiB in this fixture. The database must also update it when data changes. We did not measure the cost of those writes.The following table shows the larger gain from adding indexes to the baseline. #664 had lower medians on these common paths. These measurements do not establish that #705 is faster there. Both PRs use the same indexes for these queries.
This PR also limits the shared-frame check to two rows. That is enough to find whether another detection refers to the file. The limited query took 0.76 ms, about 111x faster than the original baseline query. The index from #664 provides the main gain. This fixture has only two rows per shared frame, so it does not measure the benefit of limiting a larger result.
Merge one database PR. #705 includes #664's index changes. Do not merge both: their migrations create the same index names from the same parent revision.
Benchmark method, implementation, and checks
Python 3.11.15 / PostgreSQL 15.19. Fixture: 2.6 million detections, 56,000 sequences, 50 active cameras/poses. Rows from different cameras are spread through the table. Medians of 21 calls after warmup include the actual Python CRUD call and database round trip. Nonempty result IDs matched across index sets. Index sets were tested in sequence; timing differences on shared paths do not isolate the extra index's effect.
The candidate query still builds 576 result objects in Python. These synthetic query benchmarks do not measure full API latency or application peak memory.
Add four indexes and matching model definitions. Declare the existing validation-queue index in the model. Deployment stops the backend before migration, so create/drop indexes in one transaction. A failed build then rolls back earlier builds. Keep the latest-detection index unfiltered so it also supports generic plans.
Five tests check index column order and filter conditions. Real PostgreSQL checks passed for migration upgrade/downgrade/re-upgrade, rollback after a name collision, and agreement between model-created and migrated indexes. The delete endpoint kept a shared file until the last reference was deleted. CI, lint, formatting, type checks, and coverage checks pass.
Net diff: +60 production/migration lines; +85 including tests. Benchmark files stay outside the diff. The four indexes occupy about 168 MiB on this fixture, including 73 MiB for the bucket-key index. Write throughput and production migration downtime were not measured.