Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
+ Coverage 94.70% 94.87% +0.17%
==========================================
Files 60 60
Lines 3266 3278 +12
==========================================
+ Hits 3093 3110 +17
+ Misses 173 168 -5
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:
|
MateoLostanlen
left a comment
There was a problem hiding this comment.
One commit per request is the right move and the rollback tests show it works. One blocker, one small nit.
Blocker: two requests for the same pose can block each other
With commit=False, the lock on a sequence row is now kept until the end of create_detection. If one frame touches sequence A then B, and another frame of the same pose touches B then A at the same time, Postgres kills one of them with deadlock detected. That request fails with a 500 and its image is already on S3. Rare, but new with this PR: the old code released the lock right after each update.
Easy fix: lock the pose for the whole transaction before the first write, e.g. SELECT pg_advisory_xact_lock(:pose_id). Requests for the same pose then wait for each other, and the lock goes away on commit or rollback.
Nit: bump_max_conf and enqueue_validation are now always called with commit=False. The default branch is the line Codecov flags. The flag can go.
Everything else looks good: each path ends with one commit or rolls back when the session closes, skipping refresh is safe with expire_on_commit=False, the bulk UPDATE in delete_sequence does the same thing as before, and I found no lock problem with the validation worker.
Commit detection and alert workflows once, bulk-unlink detections, and serialize sequence assembly per pose. A transaction advisory lock covers bbox and continuity paths after uploads, before matching. Remove unused helper commit flags.
Synthetic local PostgreSQL; three fresh workers/variant, one warmup then five rounds (15 serial or 30 concurrent requests). RSS is median worker VmHWM, including startup, excluding DB/client. Uploads are stubbed; memory is essentially unchanged.
Forced opposite-order scheduling reproduced 3/6 HTTP 500s before the lock, 0/6 after, comparing the previous PR refreshed onto main with this revision. This is a regression reproduction, not a normal error rate.
Review focus: same-pose DB work now queues; other poses use separate locks. Rollback releases the lock. S3 media is not rolled back with DB changes; continuity expiry during upload/wait can return 204 after upload.
Validation: 725 backend cases passed, one skipped; Ruff, Ty and CI pass. The existing 10 µs client timeout test passed on retry.