[SERVER-65327] AutoGetCollectionForRead should release secondary collection locks if any secondary namespace is a view or sharded, which is not supported Created: 07/Apr/22  Updated: 03/Jan/24

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Dianna Hohensee (Inactive) Assignee: Backlog - Catalog and Routing
Resolution: Unresolved Votes: 0
Labels: catalog, techdebt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Catalog and Routing
Participants:
Story Points: 3

 Description   

AutoGetCollectionForRead takes locks via its underlying AutoGetCollection, then checks the secondary namespaces and sets the value returned by isAnySecondaryNamespaceAViewOrSharded(), which indicates whether access to a secondary namespace is safe or not. However, the locks for the secondary namespaces are still held, which supports access in our low level systems (any lock invariants would pass). We should unlock the secondary namespaces.



 Comments   
Comment by Kaloian Manassiev [ 03/Jan/24 ]

Do I understand correctly that with the acquisitions API, we would be able to do what we need (i.e. drop a subset of locks/collection acquisitions)?

mihai.andrei@mongodb.com, yes this is correct. You will be able to drop individual acquisitions, which will free the locks (or any resources held) for the collections that they represent.

Comment by Mihai Andrei [ 06/Nov/23 ]

kaloian.manassiev@mongodb.com Yes, my understanding is that this is about the locked (as opposed to lock free) case. From what I remember, this is tied to $lookup pushdown in SBE (for context, $lookup in SBE requires locks for all involved namespaces, but this is not so for classic $lookup: we are only allowed to hold a lock on the outermost collection). If we take our locks and determine during query planning that we can't push down $lookup, we need a way to drop the secondary locks, but hold the lock on the main collection and proceed with planning.

> Also, with the acquisition APIs, the above scenarios are not illegal.

Do I understand correctly that with the acquisitions API, we would be able to do what we need (i.e. drop a subset of locks/collection acquisitions)?

Comment by Kaloian Manassiev [ 03/Nov/23 ]

Is this ticket really about the corner case where the locked variant of the AutoGetters took a lock on some namespace and then decided it cannot be used because it's sharded or a view? Why is the AutoGetter even usable in that case? I imagine that if this happens, we just discard the auto-getter and all the locks get freed anyways. I guess my question is who cares about this case?

Also, with the acquisition APIs, the above scenarios are not illegal.

Comment by Dianna Hohensee (Inactive) [ 22/Sep/22 ]

AutoGetCollection holds the CollectionLocks for all the namespaces, combined in a vector, while AutoGetCollectionForRead does the isAnySecondaryNamespaceAViewOrSharded check. So would require some internal and interface adjustments and additions.

Comment by Dianna Hohensee (Inactive) [ 22/Sep/22 ]

This still exists. For example, AutoGetCollectionForRead instantiates an AutoGetCollection, which has already taken any secondary namespace locks specified, and then afterward sets the flag and returns. Even if the flag is set, indicating that a secondary namespace was found to be sharded, the locks remain held when the constructor returns.

Aggregation doesn't reset the AutoGetCollection* instance based on the results of that flag, the command simply proceeds. Query uses the flag to make some decisions, like here, but I have no idea what actions are taken or not taken based on the flag.

Comment by Connie Chen [ 13/Jun/22 ]

dianna.hohensee@mongodb.com - assigning this into your next sprint, we're mainly curious about if this still applies in our current state. Feel free to throw this back into triage if you think this has changed.

Generated at Thu Feb 08 06:02:26 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.