Skip to content

Disable advisory lock transactions by default - #5526

Merged
Hawxy merged 2 commits into
JasperFx:masterfrom
Hawxy:lockyt
Sep 28, 2026
Merged

Hawxy merged 2 commits into
JasperFx:masterfrom
Hawxy:lockyt

Conversation

@Hawxy

@Hawxy Hawxy commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Hawxy
Hawxy merged commit 9189050 into JasperFx:master Sep 28, 2026
32 checks passed
@Hawxy
Hawxy deleted the lockyt branch September 28, 2026 14:52
jeremydmiller added a commit that referenced this pull request Oct 4, 2026
Closes #5567. Reported with a reproduction, a dump, and three candidate fixes, all useful.

Stopping a HotCold daemon holding many projection locks could stall 60 or 120 seconds before the locks
were released, silently, on the 9.41+ defaults. Reproduced on master: 60091 ms on iteration 10 of one
run, 60317 ms on iteration 1 of the next, nothing in two others.

The cause is in Patched.DistributedLock.Core 1.0.11, both halves visible in its shipped IL.
MultiplexedConnectionLock.Handle.DisposeAsync disposes the monitoring handle BEFORE the ReleaseAsync
that needs the shared connection for pg_advisory_unlock, and AcquireConnectionLockAsync fires its
wake-up only while registrations remain -- so the monitor worker commits to monitoring from a count the
releaser has since decremented, the last release on a connection wakes nothing, and its own 2s retry
loop re-evaluates that same false condition until the one-minute window expires. Two such connections
make it 120 s.

It is NOT a failure to interrupt the wait: NpgsqlConnection.WaitAsync honours its token promptly,
including the pre-cancelled-and-disposed-CTS shape FireStateChangedNoLock produces (0 ms) and
cancellation mid-wait (1003 ms). The interruption is never requested. And no bump fixes it -- Postgres
publishes only 1.3.2/1.3.3 and Core only 1.0.10/1.0.11, both unlisted, Weasel already references the
newest, and the fork is ahead of upstream's 1.3.1.

So AdvisoryLock moves out of Weasel.Postgresql into Marten.Events.Daemon.Coordination, where the
release path can be fixed without a two-repository release chain. Kept free of anything
Marten-specific; the intent is to move it back to Weasel once it has settled. DisposeAsync now releases
concurrently and bounds the wait with Projections.StopAndDrainTimeout, leaving anything outstanding to
finish in the background, and logs the overrun -- this path previously logged nothing unless it threw,
which is why a minute-long stop came with no explanation.

Measured over 30 iterations per configuration, the formerly stalling config is min 56 / median 792 /
p90 980 / max 988 ms. Releasing concurrently does not cost the common case, which was the risk worth
checking: all the monitoring registrations drop at once, so the intermediate wake-ups a sequential
release triggered never happen, and the fire when the count reaches zero turns out to be what unblocks
it either way. Not multiplexing also fixes it -- one registration per monitor always reaches zero --
and was rejected: 24 locks went from 1 backend connection to 24.

A non-positive or infinite ReleaseTimeout opts OUT of the bound rather than meaning "give up
immediately", matching how JasperFx reads DaemonSettings.StopAndDrainTimeout. Run against the
unguarded code only the negative case fails; zero and infinite pass either way because releases finish
in milliseconds, which the test file records so nobody reads them as coverage they aren't.

Docs: #5526 removed the only mention of UseMonitoredAdvisoryLock while flipping the transaction
default, leaving it undocumented anywhere, and the text it removed had gone stale -- on a data source
Marten owns the monitor no longer runs SELECT pg_sleep(60), it waits client-side and the backend sits
idle, which is why the stall was invisible in pg_stat_activity as well as in the logs.

Marten.Events.Daemon.Coordination.AdvisoryLock is a source-level CS0104 ambiguity for code importing
both that namespace and Weasel.Postgresql. Not a binary break; the one in-repo case drops the Weasel
import.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant