[SERVER-47817] Remove resetError command Created: 28/Apr/20  Updated: 29/Oct/23  Resolved: 28/Jan/21

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: 5.0.0

Type: Task Priority: Major - P3
Reporter: Blake Oler Assignee: Alexandre Bique (Inactive)
Resolution: Fixed Votes: 0
Labels: sharding-wfbf-day
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Documented
is documented by DOCS-14163 Investigate changes in SERVER-47817: ... Closed
Problem/Incident
causes PYTHON-2540 Test failure - test_database.TestData... Closed
Backwards Compatibility: Minor Change
Sprint: Sharding 2021-01-25, Sharding 2021-02-08
Participants:
Story Points: 1

 Comments   
Comment by Githook User [ 28/Jan/21 ]

Author:

{'name': 'Alexandre Bique', 'email': 'alexandre.bique@mongodb.com', 'username': 'abique'}

Message: SERVER-47817 Remove resetError command
Branch: master
https://github.com/mongodb/mongo/commit/e74759918118887469fe1657bb90e415f5640456

Comment by Alexandre Bique (Inactive) [ 25/Jan/21 ]

I had a look, I've adapted the tests, and scheduled a build: https://spruce.mongodb.com/version/600e8dfe3e8e8657600b7cc5/tasks

Comment by Alexandre Bique (Inactive) [ 25/Jan/21 ]

Sure I can take an second look.
Out of curiosity, why didn't we deprecate getLastError() as well? Is it the plan to keep it around forever?

Comment by Sergi Mateo Bellido [ 25/Jan/21 ]

Hi blake.oler ,

Thanks for the context! Now I understand what we misunderstood: we thought that the only way to reset an error was through the resetError command, but in fact the error is refreshed after each write operation. That's the reason why we didn't understand how we could remove resetError but keep getLastError. Now everything is clear, thanks again!

alexandre.bique Can you take a look at the jstests that use resetError? I remember this one.

Comment by Blake Oler [ 22/Jan/21 ]

alexandre.bique The work you've done so far on the ticket is exactly what I was expecting! Good job.

To dig in a bit into the context,

getLastError is still used by very old drivers to send writes as OP_INSERT, OP_UPDATE, and OP_DELETE in order to wait for write concern. Accordingly, we need to keep around getLastError. These uses of getLastError, however, don't necessitate resetError. resetError itself has been deprecated since MongoDB 1.6, which is more than a decade old. The removal of resetError was specifically requested by Andy Schwerin during the scope review for PM-1824.

In accordance, removing resetError itself has been approved by multiple major stakeholders across server, cloud, and drivers.

alexandre.bique, sergi.mateo-bellido, what is the core conclusion from your investigation? If it's that we needed to remove getLastError too, then it's fine for you to pick back up and only remove resetError. If the issue is that the work involved simply takes too long, then we can story-point the ticket and place it into a sprint.

Comment by Alexandre Bique (Inactive) [ 22/Jan/21 ]

Code review with the changes I had: https://mongodbcr.appspot.com/756210048

Comment by Alexandre Bique (Inactive) [ 22/Jan/21 ]

Hi blake.oler ,

I've investigated the issue with sergi.mateo-bellido. We've found that:

  • resetError isn't a no-op, it resets the error that one can retrieve using getLastError
  • we believe that getLastError should be removed together with resetError
  • we believe that some additionnal infrastructure in the C++ codebase related to getLastError should be removed, which might imply some more refactoring including error handling
  • many tests are to be reworked and we should ensure that the error testing is properly done.

To conclude the scope of this ticket seems much wider.

I've submitted this patch: https://spruce.mongodb.com/version/6009a4ae32f417231ddf7238/tasks which does not pass the tests.

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