Commit Graph
1 Commits
Author SHA1 Message Date
Isaac ConnorandClaude Opus 5 1f70965a98 fix: retry deadlocked queries with bounded backoff instead of failing them
zmDbDo, zmDbDoInsert and zmDbDoUpdate all decided what to do with a failed
query by testing for ER_LOCK_WAIT_TIMEOUT alone, which got both halves of
lock contention wrong:

- ER_LOCK_DEADLOCK was not retried at all. InnoDB resolves a deadlock by
  rolling one side back and expects that side to re-run; instead the query
  was logged and abandoned. Event creation goes through zmDbDoInsert, which
  is where this actually bites.

- ER_LOCK_WAIT_TIMEOUT re-ran immediately and forever, with no delay and no
  attempt limit, so two writers deadlocking against each other kept
  colliding on the same schedule.

Both now go through one retry decision: five attempts with a jittered
50ms-doubling backoff, then give up and report. The jitter is what stops
two contending sessions waking together and repeating the deadlock.

The wait happens under db_mutex, which every other database user in the
process is blocked on, so the budget is deliberately small -- about 3.1s
across all five attempts. That is still far less than the unbounded
ER_LOCK_WAIT_TIMEOUT loop it replaces, where each round costs a full
innodb_lock_wait_timeout. The change in behaviour is that a query which
would eventually have won after many minutes is now abandoned; it is
reported at Error with the attempt count.

The error is now logged once, when giving up, rather than on every round.
mysql_errno is read next to mysql_error rather than after the logging
call, so it cannot be clobbered in between.

Two other things, both small and both in the same area:

zmDbEscapeString called mysql_real_escape_string unconditionally, and that
reads the character set off the connection, so a closed handle sends it
into freed state. It now falls back to escaping the injection-relevant
characters itself. That fallback is only correct because the connection is
utf8mb4, where no byte of a multi-byte sequence is ASCII and so no sequence
can absorb a trailing backslash; the comment says so, since it would be
wrong for a character set like GBK. It deliberately does not take
db_mutex to read the flag: the logger calls this from Error(), and
zmDbFetch reaches Error() while holding db_mutex, so locking here would
self-deadlock the process on any failed query.

zm_rtsp_server was the one daemon closing the database without stopping
the queue that writes through it. Fixed at the call site rather than
inside zmDbClose, which holds db_mutex while the queue thread needs that
same mutex to drain -- joining it from in there would deadlock.

Tests: tests/zm_db_contention.cpp covers the retry budget only, 2011
assertions. Verified it fails when the budget is removed. The retry loop
itself needs a database and two contending sessions and is NOT covered;
it wants verifying against a real server under contention before this is
relied on. Full suite 12171 assertions in 133 test cases. Builds clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-09-04 07:15:30 -04:00