Skip to content

Commit ebc2c91

Browse files
committed
store: Fix deadlock between AcquireHaltLock and processLTXStreamFrame
Moving the halt lock check after receiving the LTX increases the probability of an existing deadlock: - AcquireHaltLock stores remoteHaltLock and calls WaitPosExact, i.e. it waits for processLTXStreamFrame to process pending LTXs - processLTXStreamFrame sees remoteHaltLock != nil (this is more likely now, since it receives the LTX first) - It incorrectly assumes the remote lock is stale and tries to clear it, since we don't expect new LTXs from the primary while we hold the halt lock - UnsetRemoteHaltLock deadlocks in Recover, because processLTXStreamFrame already called AcquireWriteLock - WaitPosExact deadlocks, because it's waiting for processLTXStreamFrame, which is deadlocked Instead of trying to clear the remote lock locally, processLTXStreamFrame will perform recovery so the LTX can be applied. If the remote lock was stale, we'll now only find out when committing a transaction, as the primary will reject the remote commit. Deferring the error is an acceptable trade-off, as the halt lock is intended to be short-lived.
1 parent 62a68cc commit ebc2c91

1 file changed

Lines changed: 8 additions & 6 deletions

File tree

store.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,14 +1583,16 @@ func (s *Store) processLTXStreamFrame(ctx context.Context, frame *LTXStreamFrame
15831583
}
15841584
defer guardSet.Unlock()
15851585

1586-
// If we receive an LTX file while holding the remote HALT lock then the
1587-
// remote lock must have expired or been released so we can clear it locally.
1586+
// If we receive an LTX file while holding the remote HALT lock, then the remote lock
1587+
// might have expired or been released, and we could clear it locally. However, we might
1588+
// have recently acquired the remote lock and are waiting to catch up with the primary.
15881589
//
1589-
// We also hold the local WRITE lock so a local write cannot be in-progress.
1590+
// In both cases, we hold the local WRITE lock so a local write cannot be in-progress.
1591+
// We'll perform recovery so the LTX can be applied, but not clear the remote lock.
15901592
if haltLock := db.RemoteHaltLock(); haltLock != nil {
1591-
TraceLog.Printf("[ProcessLTXStreamFrame.Unhalt(%s)]: replica holds HALT lock but received LTX file, unsetting HALT lock", db.Name())
1592-
if err := db.UnsetRemoteHaltLock(ctx, haltLock.ID); err != nil {
1593-
return fmt.Errorf("release remote halt lock: %w", err)
1593+
TraceLog.Printf("[ProcessLTXStreamFrame.Recover(%s)]: replica holds HALT lock but received LTX file, performing recovery", db.Name())
1594+
if err := db.recover(ctx); err != nil {
1595+
return fmt.Errorf("recover: %w", err)
15941596
}
15951597
}
15961598

0 commit comments

Comments
 (0)