[SERVER-30049] applyOperation_inlock() allows exceptions from Collection::insertDocument() to percolate to caller Created: 07/Jul/17  Updated: 30/Oct/23  Resolved: 25/Jul/17

Status: Closed
Project: Core Server
Component/s: Replication
Affects Version/s: 3.4.4
Fix Version/s: 3.2.17, 3.4.7, 3.5.11

Type: Bug Priority: Major - P3
Reporter: Daniel Gottlieb (Inactive) Assignee: Benety Goh
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Related
related to SERVER-30530 applyOps triggers invariant on WCE wh... Closed
is related to SERVER-23326 applyOps should use a single WriteUni... Closed
is related to SERVER-18982 Apply replicated inserts as inserts Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Completed:
Steps To Reproduce:

ops = [];
for (var i = 0; i < 10; ++i) {
    ops.push({op: "i", o: {_id: i}, ts: Timestamp(), v: 2, ns: "test.test"});
}
ops.push({ns: "test.$cmd", op: "c", o: {applyOps: []}});
db.test.drop();
db.test.insert({});
db.adminCommand({setParameter: 1, traceWriteConflictExceptions: true});
db.adminCommand({configureFailPoint: 'WTWriteConflictException', mode: {activationProbability: 0.05}});
db.adminCommand({applyOps: ops})

Sprint: Repl 2017-07-31
Participants:
Case:

 Description   

This bug affects 3.4 and although some code has changed in master, I believe it still exists in the same way. This ticket will reference the 3.4.4 code.

The bug is in the contract between a call from _applyOps to applyOperation_inlock for a non-upserting insert. If applyOperation_inlock is wrong, the return for other operations may need to be audited.

Consider a WriteConflictException (WCE) thrown in the collection->insert call here: https://github.com/mongodb/mongo/blob/r3.4.4/src/mongo/db/repl/oplog.cpp#L846-L861

In this case applyOperation_inlock is converting the WCE (which extends from DBException) into a Status with the appropriate code of 112 (WCE).

However the caller from _applyOps is anticipating WCE's to be thrown as it's wrapped in a WCE retry loop:
https://github.com/mongodb/mongo/blob/r3.4.4/src/mongo/db/catalog/apply_ops.cpp#L167-L177

The status with error code 112 increments the errors counter:
https://github.com/mongodb/mongo/blob/r3.4.4/src/mongo/db/catalog/apply_ops.cpp#L194-L197

Which results in the applyOps command returning ErrorCode: 8, UnknownError:
https://github.com/mongodb/mongo/blob/r3.4.4/src/mongo/db/catalog/apply_ops.cpp#L252-L254

As far as I know, engaging the retry loop in _applyOps would be correct behavior.

For interested parties, WCE's can (infrequently) be thrown on reads/writes other than concurrent access to the same document. These causes are usually memory pressure related.



 Comments   
Comment by Githook User [ 11/Aug/17 ]

Author:

{'name': 'Benety Goh', 'email': 'benety@mongodb.com'}

Message: SERVER-30049 applyOperation_inlock() allows exceptions from Collection::insertDocument() to percolate to caller

This specifically addresses the case where a WriteConflictException is thrown from insertDocument().
Converting this exception into a status will not allow the write conflict retry loops in callers to
work properly.

(cherry picked from commit f0d41183e735546c83ff96d7d5afc11b9c94cb9f)
Branch: v3.2
https://github.com/mongodb/mongo/commit/8c4994913a437d2f26e9596d60a63cd235a35f02

Comment by Githook User [ 26/Jul/17 ]

Author:

{'email': 'benety@mongodb.com', 'username': 'benety', 'name': 'Benety Goh'}

Message: SERVER-30049 applyOperation_inlock() allows exceptions from Collection::insertDocument() to percolate to caller

This specifically addresses the case where a WriteConflictException is thrown from insertDocument().
Converting this exception into a status will not allow the write conflict retry loops in callers to
work properly.

(cherry picked from commit f0d41183e735546c83ff96d7d5afc11b9c94cb9f)
Branch: v3.4
https://github.com/mongodb/mongo/commit/026a71c19c3f4604e8bc134e74dede28f175c063

Comment by Githook User [ 25/Jul/17 ]

Author:

{'email': 'benety@mongodb.com', 'username': 'benety', 'name': 'Benety Goh'}

Message: SERVER-30049 applyOperation_inlock() allows exceptions from Collection::insertDocument() to percolate to caller

This specifically addresses the case where a WriteConflictException is thrown from insertDocument().
Converting this exception into a status will not allow the write conflict retry loops in callers to
work properly.
Branch: master
https://github.com/mongodb/mongo/commit/f0d41183e735546c83ff96d7d5afc11b9c94cb9f

Comment by Benety Goh [ 24/Jul/17 ]

For secondary application, we will retry in the event of WriteConflictException because we explicitly convert the WriteConflict status from applyOperation_inlock() back into an exception here:

https://github.com/mongodb/mongo/commit/4d5b80ab7f0690908475919fbd52ec4459b363f7#diff-0213f2250597600264ce8a3ee9b3943cR176

In the applyOps code path, there is no similar check on the result of applyOperation_inlock() which led us to the issue in this ticket.

Comment by Benety Goh [ 24/Jul/17 ]

Please ignore the previous comment on SERVER-21229. The try-catch block was introduced in SERVER-18982 (released in 3.2):

https://github.com/mongodb/mongo/commit/4d5b80ab7f0690908475919fbd52ec4459b363f7#diff-c64a61c81bff3e9fbae542a82463b76dR639

Comment by Roderick Condell [ 24/Jul/17 ]

Hi All,

Thanks for the updates on this.

Best,
Rod

Comment by Benety Goh [ 24/Jul/17 ]

You're right. 3.2 backport is still applicable.

Comment by Daniel Gottlieb (Inactive) [ 24/Jul/17 ]

That's a hard diff to parse, but I think the try/catch predates that patch: https://github.com/mongodb/mongo/commit/2e153f35f45e284d066210792b7b231b033baaa8#diff-c64a61c81bff3e9fbae542a82463b76dL772

Comment by Benety Goh [ 24/Jul/17 ]

try-catch block was introduced in SERVER-21229:

https://github.com/mongodb/mongo/commit/2e153f35f45e284d066210792b7b231b033baaa8#diff-c64a61c81bff3e9fbae542a82463b76dR799

This issue affects 3.4 but not 3.2.

Generated at Thu Feb 08 04:22:31 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.