[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: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||
| Backport Completed: | |||||||||||||||||||||
| Steps To Reproduce: |
|
||||||||||||||||||||
| Sprint: | Repl 2017-07-31 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Case: | (copied to CRM) | ||||||||||||||||||||
| 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: The status with error code 112 increments the errors counter: Which results in the applyOps command returning ErrorCode: 8, UnknownError: 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: This specifically addresses the case where a WriteConflictException is thrown from insertDocument(). (cherry picked from commit f0d41183e735546c83ff96d7d5afc11b9c94cb9f) |
| Comment by Githook User [ 26/Jul/17 ] |
|
Author: {'email': 'benety@mongodb.com', 'username': 'benety', 'name': 'Benety Goh'}Message: This specifically addresses the case where a WriteConflictException is thrown from insertDocument(). (cherry picked from commit f0d41183e735546c83ff96d7d5afc11b9c94cb9f) |
| Comment by Githook User [ 25/Jul/17 ] |
|
Author: {'email': 'benety@mongodb.com', 'username': 'benety', 'name': 'Benety Goh'}Message: This specifically addresses the case where a WriteConflictException is thrown from insertDocument(). |
| 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: 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 |
| Comment by Roderick Condell [ 24/Jul/17 ] |
|
Hi All, Thanks for the updates on this. Best, |
| 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 This issue affects 3.4 but not 3.2. |