[SERVER-41918] CollectionBulkLoader does not anticipate exceptions from MultiIndexBlock Created: 25/Jun/19 Updated: 29/Oct/23 Resolved: 09/Sep/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | None |
| Fix Version/s: | 4.0.13, 4.2.1, 4.3.1 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Eric Milkie | Assignee: | Mihai Andrei |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | former-quick-wins, former-robust-initial-sync | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||
| Operating System: | ALL | ||||||||||||
| Backport Requested: |
v4.2, v4.0
|
||||||||||||
| Sprint: | Repl 2019-09-09 | ||||||||||||
| Participants: | |||||||||||||
| Description |
|
Currently, CollectionBulkLoaderImpl uses CollectionBulkLoaderImpl::_runTaskReleaseResourcesOnFailure() to wrap function calls into other components such as the MultiIndexBlock index builder component. CollectionBulkLoaderImpl::_runTaskReleaseResourcesOnFailure() assumes no exceptions will reach its top level, since it catches all exceptions and immediately calls std::terminate() when they are caught. Since some MultiIndexBlock functions can uassert, I believe we ought to convert exceptions into Statuses either in CollectionBulkLoaderImpl at its call sites, or within the MultiIndexBlock functions themselves. |
| Comments |
| Comment by Githook User [ 26/Sep/19 ] |
|
Author: {'name': 'Mihai Andrei', 'email': 'mihai.andrei@mongodb.com'}Message: (cherry picked from commit 6dc4461c0db2954da0a43d3934bb6c97ac02fd8e) |
| Comment by Githook User [ 13/Sep/19 ] |
|
Author: {'name': 'Mihai Andrei', 'email': 'mihai.andrei@mongodb.com'}Message: |
| Comment by Githook User [ 09/Sep/19 ] |
|
Author: {'name': 'Mihai Andrei', 'email': 'mihai.andrei@mongodb.com'}Message: |
| Comment by Eric Milkie [ 28/Aug/19 ] |
|
After in-person discussion, we will implement this ticket by adorning MultiIndexBlock's calls of functions that might throw with try-catch's that convert exceptions to Statuses. |
| Comment by Matthew Russotto [ 01/Jul/19 ] |
|
It appears the intention is that the MultiIndexBlock functions which return “Status” values are not supposed to uassert. In theory this should hold all the way down the chain; that is, a function which returns a Status should be able to be trusted not to uassert instead, and it appears MultiIndexBlock assumes this. In practice there is no way to enforce this (statically) in the language, so it is not reliable. The MultiIndexBlock already catches some lower-level uasserts and returns them as statuses, and sometimes it special-cases some of the errors, which CollectionBulkLoaderImpl wouldn’t know how to do. So we should probably enforce this constraint at the MultiIndexBlock level. Any outside function called should be wrapped in a try/catch. |