[SERVER-46800] Fix doReplSetReconfig to always return a status on error Created: 11/Mar/20  Updated: 17/Mar/20  Resolved: 17/Mar/20

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

Type: Bug Priority: Major - P3
Reporter: Vesselina Ratcheva (Inactive) Assignee: William Schultz (Inactive)
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-46742 Factor out logic to wait for config c... Closed
Operating System: ALL
Sprint: Repl 2020-03-23
Participants:

 Description   

The contract for doReplSetReconfig (as observed by all callers) is that it should always return a bad status on error and never throw (no callers catch exceptions). There is an exception to this when waiting for the config to be replicated to a majority (here) - that should be changed to a status return.



 Comments   
Comment by William Schultz (Inactive) [ 17/Mar/20 ]

This should be addressed by the changes from SERVER-46742. That patch moves the logic to wait for config commitment into a separate method which now always returns a Status in case of an error. It does not throw an exception. Also note that doReplSetReconfig doesn't actually call this waiting logic anymore. It is now called at the command processing layer.

Comment by Judah Schvimer [ 11/Mar/20 ]

I agree throwing an exception can be preferable to a status. I am wary of functions that do both though and it currently can return a status.

Comment by Siyuan Zhou [ 11/Mar/20 ]

I agree with william.schultz that this will be addressed in SERVER-46742. judah.schvimer, let's wait to see if this ends up being a dup.

We prefer to throw an exception on errors for user commands, but it seems doReplSetReconfig() is also called internally, for example on step up. Althought the code linked by Vessy isn't called outside of commands, it's a good idea to keep the contract consistent.

Comment by Judah Schvimer [ 11/Mar/20 ]

siyuan.zhou, does this pose problems for safe reconfig? Does it need to be backported?

Comment by William Schultz (Inactive) [ 11/Mar/20 ]

vesselina.ratcheva I think this will end up being addressed by SERVER-46742.

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