[SERVER-62101] Aggregation can run lock-free and expects a ViewCatalog access separate from the AutoGet*MaybeLockFree to always return a valid ViewCatalog -- not guaranteed Created: 16/Dec/21 Updated: 29/Oct/23 Resolved: 05/Jan/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.3.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Dianna Hohensee (Inactive) | Assignee: | Dianna Hohensee (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | read-only-views | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||
| Operating System: | ALL | ||||||||||||
| Sprint: | Execution Team 2021-12-27, Execution Team 2022-01-10 | ||||||||||||
| Participants: | |||||||||||||
| Linked BF Score: | 145 | ||||||||||||
| Description |
|
This code does DatabaseHolder::getViewCatalog()->resolveView without checking that getViewCatalog doesn't return a nullptr. It makes this assumption because the code is gated by an autoGet->getView check. Two solution here. 1) Aggregation can error when it finds the ViewCatalog no longer exists. (I rather like the simplicity). This means the collection didn't exist when the command started, and then the view went away somehow during the command. Aggregate is just about to drop locks anyway, which means anything can happen after view resolution 2) Lock-free operations must support ViewCatalog::resolveView with the same ViewCatalog used to fetch the view – likely save the ViewCatalog shared_ptr on the AutoGet*LockFree, if we find a view.
|
| Comments |
| Comment by Githook User [ 04/Jan/22 ] |
|
Author: {'name': 'Dianna Hohensee', 'email': 'dianna.hohensee@mongodb.com', 'username': 'DiannaHohensee'}Message: |
| Comment by Dianna Hohensee (Inactive) [ 04/Jan/22 ] |
|
The failures started on |
| Comment by Pavithra Vetriselvan [ 17/Dec/21 ] |
|
We should consider going with the first solution. We should also try to understand why this started happening recently. |