[SERVER-65955] Shell's special query path for exhaust queries or "$cmd" queries trips tassert on negative limit Created: 26/Apr/22  Updated: 29/Oct/23  Resolved: 11/Jun/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Mindaugas Malinauskas Assignee: David Storch
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related
related to SERVER-66047 Parameter 'limit' is not supported fo... Closed
is related to SERVER-58491 Consolidate the C++ native cursor imp... Backlog
is related to SERVER-20770 Consolidate the two code paths for ru... Backlog
is related to SERVER-67116 Remove support for applyOps command p... Closed
Backwards Compatibility: Fully Compatible
Sprint: QE 2022-06-13
Participants:
Linked BF Score: 15

 Description   

The shell currently has two separate paths for running find commands: the DBCommandCursor path which is implemented primarily in the JavaScript layer, and the C++ native path. Historically, the former was used for find commands and the latter was used for legacy OP_QUERY find. However, OP_QUERY find is no longer supported by the shell. The only cases in which the C++ path currently get used are for queries against the "$cmd" namespace and for exhaust queries.

The C++ native path is broken when given a negative limit. This trips a tassert in the shell. Examples:

MongoDB Enterprise > db.$cmd.foo.find().limit(-3)
{"t":{"$date":"2022-06-08T14:47:06.679Z"},"s":"E",  "c":"ASSERT",   "id":4457000, "ctx":"js","msg":"Tripwire assertion","attr":{"error":{"code":5746103,"codeName":"Location5746103","errmsg":"DBClientCursor limit must be non-negative"},"location":"{fileName:\"src/mongo/client/dbclient_cursor.cpp\", line:669, functionName:\"operator()\"}"}}
Error: DBClientCursor limit must be non-negative
 
MongoDB Enterprise > db.$cmd.foo.findOne()
{"t":{"$date":"2022-06-08T14:47:11.579Z"},"s":"E",  "c":"ASSERT",   "id":4457000, "ctx":"js","msg":"Tripwire assertion","attr":{"error":{"code":5746103,"codeName":"Location5746103","errmsg":"DBClientCursor limit must be non-negative"},"location":"{fileName:\"src/mongo/client/dbclient_cursor.cpp\", line:669, functionName:\"operator()\"}"}}
Error: DBClientCursor limit must be non-negative :
DBQuery.prototype._exec@src/mongo/shell/query.js:149:40
DBQuery.prototype.hasNext@src/mongo/shell/query.js:302:10
DBCollection.prototype.findOne@src/mongo/shell/collection.js:267:17
@(shell):1:13
 
MongoDB Enterprise > db.c.find().addOption(DBQuery.Option.exhaust).limit(-10)
{"t":{"$date":"2022-06-08T14:47:31.846Z"},"s":"E",  "c":"ASSERT",   "id":4457000, "ctx":"js","msg":"Tripwire assertion","attr":{"error":{"code":5746103,"codeName":"Location5746103","errmsg":"DBClientCursor limit must be non-negative"},"location":"{fileName:\"src/mongo/client/dbclient_cursor.cpp\", line:669, functionName:\"operator()\"}"}}
Error: DBClientCursor limit must be non-negative

The work for this ticket is to fix the shell so that these queries work as expected, which will resolve the linked build failure.

As far as I can tell, there is no reason to use the C++ path for "$cmd" namespaces. I think this relates to some old behavior around pseudo-commands which is no longer relevant. We should just use the normal DBCommandCursor path in this case.

We still need to use the C++ native path for exhaust queries. (Note, however, that SERVER-20770 and SERVER-58491 suggest consolidating the two code paths in the shell.) A good fix for this problem would be to add support for exhaust in the new DBClientBase::find() API and then migrate the shell from DBClientBase::query_DEPRECATED() to DBClientBase::find().



 Comments   
Comment by Githook User [ 10/Jun/22 ]

Author:

{'name': 'David Storch', 'email': 'david.storch@mongodb.com', 'username': 'dstorch'}

Message: SERVER-65955 Migrate shell exhaust path onto modern internal client API

In doing so, this also fixes the shell's C++ native query
path to correctly handle negative limit. The patch also
includes additional preparatory work for deleting the
query_DEPRECATED() internal client API.
Branch: master
https://github.com/mongodb/mongo/commit/f6b83615b8a3435193e501424bf4b9b91f9e8a1d

Comment by David Storch [ 08/Jun/22 ]

I've updated the title and description to provide a more accurate description of the bug as well as my planned fix.

Comment by Connie Chen [ 26/Apr/22 ]

We should investigate what dependencies DB.fsyncUnlock() has (if any)

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