[yordis]

• When a lock owner exited while the connection could not answer, the unlock was never retried, so the lock stayed held and the subscription could not move to another node.
• Raised in the review of fix(subscriptions): tell lock owners their lock is gone before the connection finishes closing #27.

[cursor]

PR Summary

Medium Risk
Changes lock-owner teardown in the event-store advisory lock GenServer; incorrect retry behavior could leave locks held longer or affect subscription fencing under connection stress.

Overview
When a lock owner process dies, advisory lock cleanup now goes through release_held_lock (same as an explicit release) instead of calling release_lock immediately. That queues the unlock on the server’s released list and drives verify_sessions retries when the lock DB pool cannot answer, so the key is not left stuck in PostgreSQL.

Tests are refactored with start_impatient_lock_server (tiny pool + short query timeout) and a new case covers owner shutdown while the only connection is held, asserting the lock eventually becomes acquirable again.

^{Reviewed by Cursor Bugbot for commit 52f163f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: eeba5f12-0bee-4a50-b403-09af60d0184e

Commits

Reviewing files that changed from the base of the PR and between 46533ec and e24ad0f.

Files selected for processing: 2

• lib/event_store/advisory_locks.ex
• test/advisory_locks_test.exs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

When a lock owner terminates, the handler now passes the full lock to release_held_lock/2. The tests verify that the release remains pending while the pooled connection is held and that the lock becomes available after the connection is released.

Changes

Owner Lock Release

Layer / File(s)	Summary
Route owner termination through lock release lib/event_store/advisory_locks.ex, test/advisory_locks_test.exs	The owner-termination handler now uses release_held_lock/2. Tests exercise pending unlocks while the pooled connection is held and verify that locks become available after it is released.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

• TrogonStack/commanded_eventstore issue 43: Describes the owner-down unlock failure addressed by routing the lock through the retry path.
• TrogonStack/commanded_eventstore issue 47: Reports missing retry re-arming coverage; this change adds a test for owner termination while the connection is busy.

Possibly related PRs

• TrogonStack/commanded_eventstore#25: Adds the shared query and failed-unlock retry path used by this change.
• TrogonStack/commanded_eventstore#27: Covers advisory-lock session-loss handling, which is related to owner-termination recovery.
• TrogonStack/commanded_eventstore#24: Introduces session-aware locks and pending releases used by this change.

Suggested reviewers: slashdotdash, vheathen

Merge Risk: Minimal · up to e24ad

The change addresses the stranded-lock case, and no actionable merge-blocking risk remains after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the subscription fix and the retry behavior for unlocking a lock after its owner exits.
Description check		The description directly explains the failed unlock retry case and its impact on lock ownership and subscription movement.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit watched the lock hold tight,
While busy connections blocked its flight.
The owner left; the release stayed,
Till free connections cleared the way.
The lock sprang loose, and hopped away.}

_{Comment @coderabbitai help to get the list of available commands.}