[SERVER-34947] libmongodb_capi errors when aggregating with $out Created: 11/May/18  Updated: 29/Oct/23  Resolved: 16/May/18

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

Type: Task Priority: Major - P3
Reporter: Ross Lawley Assignee: Henrik Edin
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Platforms 2018-05-21
Participants:

 Description   

Running the following command via the embedded java driver: 

{ "aggregate" : "com.mongodb.embedded.client.CrudTest", "pipeline" : [{ "$sort" : { "x" : 1 } }, { "$match" : { "_id" : { "$gt" : 1 } } }, { "$out" : "other_test_collection" }], "cursor" : { }, "$db" : "JavaDriverTest", "$readPreference" : { "mode" : "primaryPreferred" } } 

Throws a:

{ "ok" : 0.0, "errmsg" : "insert for $out failed: { ok: 0.0, errmsg: "no such command: 'getlasterror'", code: 59, codeName: "CommandNotFound" }", "code" : 16996, "codeName" : "Location16996" }

The server logging callback returns:

11:07:23.359 [main] INFO  org.mongodb.driver.embedded.server - STORAGE   [initandlisten] createCollection: JavaDriverTest.tmp.agg_out.1 with generated UUID: a57bc19d-7ee9-4495-8a00-16100e35d91d
11:07:23.375 [main] INFO  org.mongodb.driver.embedded.server - COMMAND   [initandlisten] CMD: drop JavaDriverTest.tmp.agg_out.1
11:07:23.375 [main] INFO  org.mongodb.driver.embedded.server - STORAGE   [initandlisten] Finishing collection drop for JavaDriverTest.tmp.agg_out.1 (a57bc19d-7ee9-4495-8a00-16100e35d91d). 



 Comments   
Comment by Githook User [ 16/May/18 ]

Author:

{'email': 'henrik.edin@mongodb.com', 'username': 'henrikedin', 'name': 'Henrik Edin'}

Message: SERVER-34947 Make getLastError available in embedded.

$out is using it.
Branch: master
https://github.com/mongodb/mongo/commit/05640e4e0752370650ea2f3c950b7a493b57dc46

Comment by Henrik Edin [ 15/May/18 ]

How about doing both? We enable getLastError to be safe and we fix this case where it seems not to be needed and we can instead check the result of insert.

Comment by Andrew Morrow (Inactive) [ 15/May/18 ]

I'm fairly sure we should go with re-supporting getLastError in embedded. My reasoning is that it may be the case that $out isn't the only path that reaches into a getLastError call in some way. Rather than playing whack-a-mole, lets just support it. Next questions to my mind are whether the current implementation of the command drags in any unwanted dependencies, and, if so, should we invest in rolling an embedded specific version, as we do for isMaster

Comment by Charlie Swanson [ 15/May/18 ]

I don't think we need to worry too much about preserving the particular error message or code here. Changing the code might involve changing some tests, but I don't think anyone does/should rely on either. As long as the error still details what went wrong that should be fine.

Also - it looks like this is actually trying to create an index, but it does so by inserting into system.indexes... Perhaps there's a new/better way to do that?

Comment by Henrik Edin [ 15/May/18 ]

Re-support getlasterror is trivial. But redbeard0531 suggested to instead look at the result of the insert instead of using getlasterror. We can for example expose an insertWithResult() on the DBClient but that would change the assert message here: 

https://github.com/mongodb/mongo/blob/6d2de545a7cfcf4ab23dcf73426a1d50896d6d0c/src/mongo/db/pipeline/document_source_out.cpp#L134-L142

I'm willing to do the change, or forward this ticket to Query. Unless we want to play it super safe and just restore getlasterror for embedded.

Comment by Andrew Morrow (Inactive) [ 11/May/18 ]

Thanks charlie.swanson - We will give it some thought, but most likely the path of least resistance is to re-support getlasterror in this context. If this path reaches it, there are probably others that we haven't found yet.

Comment by Charlie Swanson [ 11/May/18 ]

acm Check out this code. I'm unfamiliar with the various interfaces of the DBDirectClient, but if there's a better interface that returns an error we should probably use that. We could also consider a switch to a raw PlanExecutor or some other path to insert, but that has some broader implications, including changes to how an $out will appear in things like the profiler and the logs.

Comment by Andrew Morrow (Inactive) [ 11/May/18 ]

Apparently, running with $out somehow invokes the getlasterror command, which mobile explicitly doesn't support. david.storch or charlie.swanson could we get some insight from query on this issue? Why is $out trying to invoke the getlasterror command?

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