[SERVER-48144] waitUntilDurable should not take a mutex before taking locks Created: 12/May/20  Updated: 29/Oct/23  Resolved: 20/May/20

Status: Closed
Project: Core Server
Component/s: Storage
Affects Version/s: None
Fix Version/s: 4.4.0-rc7, 4.7.0

Type: Bug Priority: Major - P3
Reporter: Dianna Hohensee (Inactive) Assignee: Dianna Hohensee (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Duplicate
is duplicated by SERVER-48125 Stepdown can deadlock with storing la... Closed
Related
related to SERVER-80432 waitForSession can miss JournalListen... Closed
is related to SERVER-48143 Remove support for 'fsync' flag in wr... Open
Backwards Compatibility: Fully Compatible
Backport Requested:
v4.4
Sprint: Execution Team 2020-06-01
Participants:
Linked BF Score: 22

 Description   

Replication is currently running into this deadlock issue, where the LastVote operation is using an Uninterruptible lock guard

LastVote
    waitUntilDurable
        _lastSyncMutex
            getToken
                refreshIfPrimary
                    AutoGetCollection (stuck on RSTL IX)

Stepdown (RSTL X)
    waitUntilDurable
        _lastSyncMutex (stuck on LastVote's acquisition)

Regardless of the uninterruptible lock guard, it is bad practice to take a mutex and then locks.

I recommend either of the two options (prefer #2):

1) waitUntilDurable should not call getToken under the mutex, but instead take the mutex after doing the getToken call that takes locks.
I think this is workable, because getToken (which calls refreshOplogTruncateAfterPointIfPrimary) has its own mutex for atomicity reading from the oplog and updating the oplogTruncateAfterPoint. waitUntilDurable then calls onDurable with the result of getToken after the journal flush, but replication already has protections against going backwards in time.

2) Remove waitUntilDurable's _lastSyncMutex because it is no longer needed for performance. I haven't verified that performance will be unaffected, but I believe it won't be affected from looking at the code. We've recently moved writeConcern's waitUntilDurable calls onto the async JournalFlusher thread, which has its own caller batching. Without the write callers, the only other waitUntilDurable callers are one offs for uncommon replication events. Batching doesn't seem like it needs to continue to occur in the waitUntilDurable code layer.



 Comments   
Comment by Githook User [ 21/May/20 ]

Author:

{'name': 'Dianna Hohensee', 'email': 'dianna.hohensee@mongodb.com', 'username': 'DiannaHohensee'}

Message: SERVER-48144 stop acquiring waitUntilDurable's barrier mutex before taking locks to update the oplogTruncateAfterPoint

(cherry picked from commit eae81ac42abf876039042d3f0e08e319e7e2da49)
Branch: v4.4
https://github.com/mongodb/mongo/commit/8e858d55fca372bd220fde42a3458f78bb5271e5

Comment by Githook User [ 20/May/20 ]

Author:

{'name': 'Dianna Hohensee', 'email': 'dianna.hohensee@mongodb.com', 'username': 'DiannaHohensee'}

Message: SERVER-48144 stop acquiring waitUntilDurable's barrier mutex before taking locks to update the oplogTruncateAfterPoint
Branch: master
https://github.com/mongodb/mongo/commit/eae81ac42abf876039042d3f0e08e319e7e2da49

Comment by Lingzhi Deng [ 19/May/20 ]

Here is another way to deadlock:

_heartbeatReconfigStore
    waitUntilDurable
        _lastSyncMutex
            getToken
                refreshIfPrimary
                    AutoGetCollection (stuck on RSTL IX)

Generated at Thu Feb 08 05:16:16 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.