[SERVER-40498] Writing transaction oplog entries must not take locks while holding an oplog slot Created: 05/Apr/19 Updated: 29/Oct/23 Resolved: 09/May/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | None |
| Fix Version/s: | 4.1.11 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Tess Avitabile (Inactive) | Assignee: | Siyuan Zhou |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||
| Sprint: | Repl 2019-04-22, Repl 2019-05-06, Repl 2019-05-20 | ||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||
| Linked BF Score: | 30 | ||||||||||||||||||||||||||||
| Description |
|
It is illegal to acquire new locks while holding open an oplog slot, since it can cause the deadlock described in When preparing a transaction, we reserve oplog slots here or here, then stash the locker here or here, then take a global lock here or here. When aborting a prepared transaction, we reserve an oplog slot here, then release locks here, then take a global lock here. When committing a prepared transaction, we reserve an oplog slot here, then release locks here, then take a global lock here. (Note that the global lock acquisition for writing oplog entries for unprepared transaction commit is not a problem, since the locker is already holding the global lock.) |
| Comments |
| Comment by Githook User [ 09/May/19 ] |
|
Author: {'email': 'siyuan.zhou@mongodb.com', 'name': 'Siyuan Zhou', 'username': 'visualzhou'}Message: |
| Comment by Githook User [ 04/May/19 ] |
|
Author: {'email': 'siyuan.zhou@mongodb.com', 'name': 'Siyuan Zhou', 'username': 'visualzhou'}Message: |
| Comment by Siyuan Zhou [ 04/May/19 ] |
|
tess.avitabile, you're right. Never mind, I was fooled by the two-phase locking. The final patch is in code review now. |
| Comment by Tess Avitabile (Inactive) [ 01/May/19 ] |
I don't understand why the lock will be released before calling OpObserver. If the lock is still in scope, it should not be released when the WUOW is committed. The only locks that get released due to two-phase locking when the WUOW is committed are the locks that have gone out of scope. I would also be interested to talk about |
| Comment by Siyuan Zhou [ 01/May/19 ] |
|
tess.avitabile and judah.schvimer, I managed to use the same lock in side transaction and to fix for preparing a transaction. However, I found another issue with commitPreparedTransaction. As found in I can think of two ways to solve this. The solutions are also considered in We could move onPreparedTransactionCommit() to before committing the storage transaction. Since we don't allow storage transaction commit to fail, both data WT transaction and oplog WT transaction will commit. To make sure the oplog write isn't visible before the data change so that afterCluterTime read are respected, we need to reserve an OplogSlot without using it and release it after both data and oplog writes commit as we will do in Alternatively, we could release the recovery unit and the locker from the WUOW and manage them manually on prepared commit, so that we only commit the recovery unit in _commitStorageTransaction() instead of committing the WUOW and have the locker outlive both data and oplog writes. We'll need storage team to weigh in on this solution. |
| Comment by Siyuan Zhou [ 19/Apr/19 ] |
|
Assigning this to me as Tess is on vacation next week. |
| Comment by Tess Avitabile (Inactive) [ 08/Apr/19 ] |
|
Before we reserve the oplog slot, we will manually make a Locker and take a global IX lock. We will pass this Locker into onTransactionPrepare(). In onTransactionPrepare(), we will pass the Locker to SideTransactionBlock, which will use it to populate the opCtx after it has stashed the current locker. |
| Comment by Tess Avitabile (Inactive) [ 05/Apr/19 ] |
|
milkie, geert.bosch, I am having trouble fixing the first case, for preparing a transaction. The prepare thread is already holding a global lock. However, we stash the transaction resources, so the OperationContext's current locker is not holding a global lock. The thread must not take a global lock because it is holding an oplog slot. But it is required to have a global lock when writing to the oplog through this dassert. I'm thinking about a few different solutions:
Do you have any suggestions? |