[SERVER-58309] Clean up benchRun()'s "safe" option, and related options Created: 06/Jul/21  Updated: 29/Oct/23  Resolved: 27/Jul/21

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

Type: Task Priority: Major - P3
Reporter: David Storch Assignee: Jennifer Peshansky (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Query Execution 2021-07-26, QE 2021-08-09
Participants:

 Description   

As far as I understand it, the "safe" option supported by benchRun() used to be a way for a caller to opt into using the getLastError() command in order to check that write operations have succeeded as expected. It also appears to have enabled some logging about the results of operations run by benchRun(), but only if "hideResults" is false or "showResult" is true. (By default, it looks like "hideResults" is true and "showResult" is false, meaning that this logging will be suppressed.)

We are preparing to remove support for the getLastError command, and in a previous work item we cleaned up benchRun() to require write commands and never use getLastError. However, we retained some getLastError-related code which is enabled by the "safe" option. For example:

https://github.com/mongodb/mongo/blob/e1f9a3922d20886691e4c22c4fa63ba81a89b3d7/src/mongo/shell/bench.cpp#L1229-L1238

I don't think this code makes sense anymore without getLastError. It looks for a field in the response document called "err", which is a format only used by the getLastError command. We have a few options to fix this:

  • Simply delete "safe" mode, as well as the accompanying options "throwGLE", "hideResults", and "showResult". This is assuming that we don't have any callers of benchRun() which use "safe" mode or the other options named above.
  • Re-implement "safe" mode so that it checks the BSONObj response received after running the write command. If there is an error, we could either log something about the error or throw an exception, depending on the value of some other parameter (e.g. "throwOnError").

Since we are trying to replace benchRun() with other perf testing tools (e.g. Genny) we probably should do something easy here. That said, I still think the code as written is worth fixing since it is simply incorrect; it makes getLastError-related checks in a world without getLastError.



 Comments   
Comment by Vivian Ge (Inactive) [ 06/Oct/21 ]

Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you!

Comment by Githook User [ 26/Jul/21 ]

Author:

{'name': 'Jennifer Peshansky', 'email': 'jennifer.peshansky@mongodb.com', 'username': 'jenniferpeshansky'}

Message: SERVER-58309 Delete benchRun's safe, hideResults, and showResult options.
Branch: master
https://github.com/mongodb/mongo/commit/104a14d151133302184522921a1a3225c8eb8824

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