[SERVER-52904] Improve implementation of mongo::Status Created: 17/Nov/20 Updated: 29/Oct/23 Resolved: 23/Aug/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 5.1.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Billy Donahue | Assignee: | Billy Donahue |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | carry-over | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Sprint: | Service arch 2020-12-28, Service Arch 2021-03-08, Service Arch 2021-03-22, Service Arch 2021-04-05, Service Arch 2021-04-19, Service Arch 2021-05-17, Service Arch 2021-05-31, Service Arch 2021-06-14, Service Arch 2021-06-28, Service Arch 2021-07-12, Service Arch 2021-08-09, Service Arch 2021-08-23, Service Arch 2021-09-20 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Story Points: | 1 | ||||||||||||||||||||
| Description |
|
Status has a lot of old code that needs a facelift. (started as follow-on work from Use boost::intrusive_ptr instead of handrolled refcounting for _error member. Don't declare operator<< for StringBuilderImpl that we don't define. Inline the ubiquitous Status::OK() function. Define inlines inside the class consistently. This was previously mixed and inconsistent. It eliminates a lot of the boilerplate from out-of-class definitions. Pass reason by std::string value when possible to save string copies! Accept anything as a reason parameter if it can direct-initialize a std::string. This eliminates fwd decl of std::stream, and eliminates the combinatoric redundancy of providing overloads for const char*, StringData, std::string, and std::stream, which also leaves a lot of viable argument types missing anyway. Remove useless const from return types and from parameters. Use std::is_..._v traits bools. Define inline-friend-defs for all == and != operators so they are applied symmetrically, and are only visible to ADL search. Allocate the empty reason() static local string so it is valid at shutdown. Remove the refCount() member, replace with a testOnlyAccess() proxy object that can retrieve the same data, but more obviously and strictly test only. Simplify ErrorInfo to a simple struct, inherit from boost::intrusive_ref_counter. No explicit atomics for refcounting, no const members, no member functions. Let Status enforce the constness and creation invariants. Status::ErrorInfo becomes just plain info. Add static Status::_createErrorInfo and Status::_parseErrorInfo functions to produce intrusive_ptr to initialize the _error member from constructors. Get rid of the risky *this = someStatus... error handling in favor of these functions. https://mongodbcr.appspot.com/709190050/ |
| 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 Billy Donahue [ 23/Aug/21 ] |
|
This change succeeded as long as I didn't try to refactor the RefCountable class. This may be worth looking into separately! |
| Comment by Githook User [ 23/Aug/21 ] |
|
Author: {'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}Message: |
| Comment by Billy Donahue [ 14/Jun/21 ] |
|
Code review https://mongodbcr.appspot.com/762610002 |
| Comment by Billy Donahue [ 19/Nov/20 ] |
|
Marking this as depending on |
| Comment by Billy Donahue [ 17/Nov/20 ] |
|
code review - https://mongodbcr.appspot.com/709190050/ - (obsolete) |