[SERVER-35804] Disallow dropping collections under config/admin via mongos Created: 26/Jun/18 Updated: 29/Oct/23 Resolved: 15/Apr/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | None |
| Fix Version/s: | 4.7.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Xiangyu Yao (Inactive) | Assignee: | Randolph Tan |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||
| Backwards Compatibility: | Minor Change | ||||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||||
| Steps To Reproduce: | See the linked BF. |
||||||||||||||||||||||||||||||||||||
| Sprint: | Sharding 2020-04-20 | ||||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||||
| Linked BF Score: | 45 | ||||||||||||||||||||||||||||||||||||
| Description |
|
This ticket makes it such that attempting to drop collections under the admin or config database through mongos will now throw an error. Attempting to call dropDatabase on admin or config will similarly throw an error. It is still possible to drop these collections when connecting directly to the config server. Original Description:
|
| Comments |
| Comment by Githook User [ 14/Apr/20 ] | |||||||||||
|
Author: {'name': 'Randolph Tan', 'email': 'randolph@10gen.com', 'username': 'renctan'}Message: | |||||||||||
| Comment by Dianna Hohensee (Inactive) [ 26/Jun/18 ] | |||||||||||
|
So this is happening because the fuzzer is targeting admin.mod1 in the _configsvrDropCollection command, which it previously created on the config server. _configsvrDropCollection is running, before createCollection, but apparently it takes a long time to get distlocks
createCollection admin.mod1
getting the distlocks for admin.mod1 and then hitting the invariant
So the fuzzer threw all the basic sanity assumptions out the window and this is really hilarious. RecoveryUnit::setTimestampReadSource can be used to reset the readConcern majority effect on the RecoveryUnit after acquiring the distlock – should use kUnset, which should let the dropCommand run without the weird majority readConcern setting. Storage is probably going to make dropCollection error on weird readConcern settings for future ease, but _configsrvDropCollection still needs fixing somehow. Esha pointed out that giving "admin" db to the high level sharding drop collection function hardcodes the config server as the target – see here and here curtesy of Esha. | |||||||||||
| Comment by Xiangyu Yao (Inactive) [ 26/Jun/18 ] | |||||||||||
|
I just put some logs in the BF comments about the last few operations before crash in that thread. It might be helpful. | |||||||||||
| Comment by Esha Maharishi (Inactive) [ 26/Jun/18 ] | |||||||||||
|
xiangyu.yao, I'm confused by the new description:
What is "the collection" in "adding an index to the collection"? Is it one of the config collections (e.g. config.collections or config.chunks), or the collection being dropped? If it's the collection being dropped, adding that index should happen on only on the shard(s) where the collection exists, not on the config server. What is the "dropCollection" in "_configsvrDropCollection finally called dropCollection"? Is it the ShardingCatalogManager's dropCollection? Where is the check for "check whether the numbers of indexes on disk and in memory were equal" coming from? | |||||||||||
| Comment by Esha Maharishi (Inactive) [ 26/Jun/18 ] | |||||||||||
|
We can update the ticket to say that, but I think it will get put into the "remove distlock" project, then. It basically requires using a separate opCtx whenever we need to do a majority read from a metadata command (dianna.hohensee, does that allow you to use a fresh RecoveryUnit for the read?) | |||||||||||
| Comment by Xiangyu Yao (Inactive) [ 26/Jun/18 ] | |||||||||||
|
Got it. So the ticket should be to change _configsvrDropCollection command so that it doesn't trigger any majority reads. | |||||||||||
| Comment by Dianna Hohensee (Inactive) [ 26/Jun/18 ] | |||||||||||
|
I suggest modifying the drop command to handle the issue, like we've done for other commands, by not setting majority on the RecoveryUnit until the end. I didn't communicate this clearly to Xiangyu. The ticket should be updated. We don't currently have a way of unsetting the readConcern on the RecoveryUnit. | |||||||||||
| Comment by Xiangyu Yao (Inactive) [ 26/Jun/18 ] | |||||||||||
|
We just need to set the readSource of the recoveryUnit from kMajorityCommitted back to kUnset. The fact that multiple operations use the same recoveryUnit is certainly not ideal but we can do this workaround. | |||||||||||
| Comment by Esha Maharishi (Inactive) [ 26/Jun/18 ] | |||||||||||
|
xiangyu.yao, do you know if it is actually possible to set the read snapshot back to 'local' after setting it to 'majority'? My understanding was that this was not currently possible. |