[SERVER-22541] Aggregation plan executors should be owned by global cursor manager Created: 09/Feb/16 Updated: 07/Jun/21 Resolved: 16/Mar/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Aggregation Framework, Querying |
| Affects Version/s: | None |
| Fix Version/s: | 3.5.5 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | J Rassi | Assignee: | Charlie Swanson |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | Query 11 (03/14/16), Query 2016-11-21, Query 2016-12-12, Query 2017-02-13, Query 2017-03-27 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Linked BF Score: | 15 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Plan executors that own a PipelineProxyStage do not hold a collection lock while they are being iterated. However, all executors that are eligible for kill notifications must hold a lock while they are being iterated, as concurrent access to PlanExecutor::_killReason for registered PlanExecutor objects is protected by the collection lock. As a result, if a plan executor owning a PipelineProxyStage executor is killed during execution, then the executor's call to read _killReason from PlanExecutor::killed() will race with the killing thread's call to write _killReason from PlanExecutor::kill(). This is undefined behavior, and potentially can result in a server crash. Aggregation cursors and plan executors should be owned by the global cursor manager, instead of by the collection's cursor manager. This correctly captures the fact that the lifetime of an aggregation cursor/executor is not tied to the lifetime of the collection, and this will prevent these cursors/executors from receiving kill notifications. The underlying plan executor owned by DocumentSourceCursor should remain owned by the associated collection's cursor manager, and should be registered for receiving invalidations and kill notifications. |
| Comments |
| Comment by Githook User [ 15/Mar/17 ] |
|
Author: {u'username': u'cswanson310', u'name': u'Charlie Swanson', u'email': u'charlie.swanson@mongodb.com'}Message: Moves registration of aggregation cursors to the global cursor manager. |
| Comment by Githook User [ 15/Mar/17 ] |
|
Author: {u'username': u'cswanson310', u'name': u'Charlie Swanson', u'email': u'charlie.swanson@mongodb.com'}Message: Removes the class 'ScopedTransaction' and moves the responsibility of |
| Comment by Githook User [ 15/Mar/17 ] |
|
Author: {u'username': u'cswanson310', u'name': u'Charlie Swanson', u'email': u'charlie.swanson@mongodb.com'}Message: Removes the class 'ScopedTransaction' and moves the responsibility of |
| Comment by Charlie Swanson [ 15/Nov/16 ] |
|
Bringing this into the current sprint since we believe it will provide a path forward for |
| Comment by J Rassi [ 22/Mar/16 ] |
|
Bumping fix version back to "3.3 Desired". This work was originally scheduled for 3.4 to fix a number of long-standing locking issues related to aggregation cursors. This work has many tech debt benefits in the area of query/agg integration, but it requires changing the namespace of aggregation cursor operations from "<db>.<collection>" to "<db>.$cmd.aggregate.<collection>". The merge cursors pipeline stage (currently run on the database primary shard for all sharded aggregations) currently only knows how to merge cursors on the original collection namespace, so it requires minor work to be namespace-agnostic for its input cursors ( At the moment, we're considering this upgrade restriction to be prohibitive, so we're unscheduling this from 3.4. Once 3.4 is released with the fix for |