[SERVER-30414] Many callers of PlanExecutor::restoreState() ignore the return value Created: 28/Jul/17 Updated: 12/Jan/18 Resolved: 11/Jan/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Aggregation Framework, Querying |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Charlie Swanson | Assignee: | Charlie Swanson |
| Resolution: | Duplicate | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Sprint: | Query 2018-01-15 | ||||||||
| Participants: | |||||||||
| Description |
|
PlanExecutor::restoreState() returns a boolean indicating whether it's safe to use. If a WriteConflictException is encountered while restoring, we may need to yield to recover. If this is the case, some event like a collection drop could invalidate the PlanExecutor. It seems like maybe it's safe to continue after this happens, since most callers immediately call PlanExecutor::getNext afterwards, which checks if it was killed and will return PlanExecutor::DEAD immediately. We should either make all callers handle the return value, or change it to return void if it's safe. It seems better to handle it - since calling restoreState() might leave you in a state where you have no locks. |
| Comments |
| Comment by Charlie Swanson [ 11/Jan/18 ] |
|
Indeed this was resolved by |
| Comment by Charlie Swanson [ 03/Aug/17 ] |
|
Pulling this into the current sprint since I think it will be resolved by |