Repository navigation
Take the trn repo lock only for logical commits - #6293
emelialei88 wants to merge 3 commits into
Conversation
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: 2/727 tests failed ⚠.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
remotecreate_twopc_generated
sc_downgrade [timeout] **quarantined**
truncatesc_offline_generated [timeout] **quarantined**
sql_logfill_autodisable [timeout]
c72eec1 to
97fc12a
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
sc_truncate [db unavailable at finish]
timepart_retro
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
fcbd9a5 to
52a8ca1
Compare
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
52a8ca1 to
9f6bd05
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
🎯 The "Why" (Intent)
Every master commit takes the trn repo mutex, so all commits run one at a time. Since 048d67f snapshot no longer uses the repo (modsnap keeps its own list); only serializable sessions and logical commits need it. Of ~51k prod databases in c2cfgdb, 49 enable serializable.
🛠️ The "What" (Critical Changes)
trn_repo_mtxonly for logical commits or when serializable is enabled.🔧 Races this exposes (fixed in their own commits)
Two places relied on the lock making commits run one at a time.
1. The master's election LSN (
rep->committed_lsn) could move backwards. A commit writes its record (gets an LSN), then saves that LSN for elections under a different lock:2. Logical live SC redo could save a restart LSN past a write it hadn't replayed. With the redo list empty, the redo thread treats the latest commit LSN as "caught up". A plain commit on another table can now move that LSN past a write to the SC table that is logged but not yet queued. Fix: read the LSN under the repo lock, which the writer holds until it is queued:
Each fix has a test (
committed_lsn_order,sc_redo_start_lsn) that fails without it and passes with it and on the old locking.📊 Results
8-node cluster, single-row autocommit inserts (one commit in flight per client), 8 writer threads, Release build (RelWithDebInfo). Both columns use the same binary, with a benchmark-only switch forcing the old locking.
LLOG, so every parent commit still takes the lock), 32 clients: 11,363 → 12,299 (+8%), from child commits no longer locking.