SERVER-50684 rename,cleanup internalAssert->iassert

This commit is contained in:
Billy Donahue 2020-11-19 17:22:13 +00:00 committed by Evergreen Agent
parent 3043162786
commit 32c4765e7a
10 changed files with 72 additions and 86 deletions

View File

@ -1,7 +1,7 @@
# Exception Architecture
MongoDB code uses the following types of assertions that are available for use:
- `uassert` and `internalAssert`
- `uassert` and `iassert`
- Checks for per-operation user errors. Operation-fatal.
- `tassert`
- Like uassert, but inhibits clean shutdown.
@ -24,21 +24,21 @@ The following types of assertions are deprecated:
- `dassert`
- Calls `invariant` but only in debug mode. Do not use!
MongoDB uses a series of `ErrorCodes` (defined in [mongo/base/error_codes.yml][error_codes_yml]) to
identify and categorize error conditions. `ErrorCodes` are defined in a YAML file and converted to
C++ files using [MongoDB's IDL parser][idlc_py] at compile time. We also use error codes to create
`Status` objects, which convey the success or failure of function invocations across the code base.
`Status` objects are also used internally by `DBException`, MongoDB's primary exception class, and
its children (e.g., `AssertionException`) as a means of maintaining metadata for exceptions. The
MongoDB uses a series of `ErrorCodes` (defined in [mongo/base/error_codes.yml][error_codes_yml]) to
identify and categorize error conditions. `ErrorCodes` are defined in a YAML file and converted to
C++ files using [MongoDB's IDL parser][idlc_py] at compile time. We also use error codes to create
`Status` objects, which convey the success or failure of function invocations across the code base.
`Status` objects are also used internally by `DBException`, MongoDB's primary exception class, and
its children (e.g., `AssertionException`) as a means of maintaining metadata for exceptions. The
proper usage of these constructs is described below.
## Considerations
When per-operation invariant checks fail, the current operation fails, but the process and
connection persist. This means that `massert`, `uassert`, `internalAssert` and `verify` only
terminate the current operation, not the whole process. Be careful not to corrupt process state by
mistakenly using these assertions midway through mutating process state. Examples of this include
`uassert`, `internalAssert` and `massert` inside of constructors and destructors.
connection persist. This means that `massert`, `uassert`, `iassert` and `verify` only
terminate the current operation, not the whole process. Be careful not to corrupt process state by
mistakenly using these assertions midway through mutating process state. Examples of this include
`uassert`, `iassert` and `massert` inside of constructors and destructors.
`fassert` failures will terminate the entire process; this is used for low-level checks where
continuing might lead to corrupt data or loss of data on disk.
@ -49,13 +49,13 @@ invoke the tripwire fatal assertion. This is useful for ensuring that operation
a test suite to fail, without resorting to different behavior during testing, and without allowing
user operations to potentially disrupt production deployments by terminating the server.
Both `massert` and `uassert` take error codes, so that all assertions have codes associated with
them. Currently, programmers are free to provide the error code by either using a unique location
number or choose from existing `ErrorCodes`. Unique location numbers are assigned incrementally and
Both `massert` and `uassert` take error codes, so that all assertions have codes associated with
them. Currently, programmers are free to provide the error code by either using a unique location
number or choose from existing `ErrorCodes`. Unique location numbers are assigned incrementally and
have no meaning other than a way to associate a log message with a line of code.
`internalAssert` provides similar functionality to `uassert`, but it logs at a higher level and
does not increment user assertion counters. We should always choose `internalAssert` over `uassert`
`iassert` provides similar functionality to `uassert`, but it logs at a higher level and
does not increment user assertion counters. We should always choose `iassert` over `uassert`
when we expect a failure, a failure might be recoverable, or failure accounting is not interesting.
@ -72,46 +72,46 @@ The inheritance hierarchy resembles:
See util/assert_util.h.
Generally, code in the server should be able to tolerate (e.g., catch) a `DBException`. Server
functions must be structured with exception safety in mind, such that `DBException` can propagate
upwards harmlessly. The code should also expect, and properly handle, `UserException`. We use
Generally, code in the server should be able to tolerate (e.g., catch) a `DBException`. Server
functions must be structured with exception safety in mind, such that `DBException` can propagate
upwards harmlessly. The code should also expect, and properly handle, `UserException`. We use
[Resource Acquisition Is Initialization][raii] heavily.
## ErrorCodes and Status
MongoDB uses `ErrorCodes` both internally and externally: a subset of error codes (e.g.,
`BadValue`) are used externally to pass errors over the wire and to clients. These error codes are
the means for MongoDB processes (e.g., *mongod* and *mongo*) to communicate errors, and are visible
to client applications. Other error codes are used internally to indicate the underlying reason for
a failed operation. For instance, `PeriodicJobIsStopped` is an internal error code that is passed
to callback functions running inside a [`PeriodicRunner`][periodic_runner_h] once the runner is
stopped. The internal error codes are for internal use only and must never be returned to clients
MongoDB uses `ErrorCodes` both internally and externally: a subset of error codes (e.g.,
`BadValue`) are used externally to pass errors over the wire and to clients. These error codes are
the means for MongoDB processes (e.g., *mongod* and *mongo*) to communicate errors, and are visible
to client applications. Other error codes are used internally to indicate the underlying reason for
a failed operation. For instance, `PeriodicJobIsStopped` is an internal error code that is passed
to callback functions running inside a [`PeriodicRunner`][periodic_runner_h] once the runner is
stopped. The internal error codes are for internal use only and must never be returned to clients
(i.e., in a network response).
Zero or more error categories can be assigned to `ErrorCodes`, which allows a single handler to
serve a group of `ErrorCodes`. `RetriableError`, for instance, is an `ErrorCategory` that includes
all retriable `ErrorCodes` (e.g., `HostUnreachable` and `HostNotFound`). This implies that an
operation that fails with any error code in this category can be safely retried. We can use
`ErrorCodes::isA<${category}>(${error})` to check if `error` belongs to `category`. Alternatively,
we can use `ErrorCodes::is${category}(${error})` to check error categories. Both methods provide
Zero or more error categories can be assigned to `ErrorCodes`, which allows a single handler to
serve a group of `ErrorCodes`. `RetriableError`, for instance, is an `ErrorCategory` that includes
all retriable `ErrorCodes` (e.g., `HostUnreachable` and `HostNotFound`). This implies that an
operation that fails with any error code in this category can be safely retried. We can use
`ErrorCodes::isA<${category}>(${error})` to check if `error` belongs to `category`. Alternatively,
we can use `ErrorCodes::is${category}(${error})` to check error categories. Both methods provide
similar functionality.
To represent the status of an executed operation (e.g., a command or a function invocation), we
use `Status` objects, which represent an error state or the absence thereof. A `Status` uses the
standardized `ErrorCodes` to determine the underlying cause of an error. It also allows assigning
a textual description, as well as code-specific extra info, to the error code for further
clarification. The extra info is a subclass of `ErrorExtraInfo` and specific to `ErrorCodes`. Look
To represent the status of an executed operation (e.g., a command or a function invocation), we
use `Status` objects, which represent an error state or the absence thereof. A `Status` uses the
standardized `ErrorCodes` to determine the underlying cause of an error. It also allows assigning
a textual description, as well as code-specific extra info, to the error code for further
clarification. The extra info is a subclass of `ErrorExtraInfo` and specific to `ErrorCodes`. Look
for `extra` in [here][error_codes_yml] for reference.
MongoDB provides `StatusWith` to enable functions to return an error code or a value without
requiring them to have multiple outputs. This makes exception-free code cleaner by avoiding
functions with multiple out parameters. We can either pass an error code or an actual value to a
`StatusWith` object, indicating failure or success of the operation. For examples of the proper
usage of `StatusWith`, see [mongo/base/status_with.h][status_with_h] and
[mongo/base/status_with_test.cpp][status_with_test_cpp]. It is highly recommended to use `uassert`
or `internalAssert` over `StatusWith`, and catch exceptions instead of checking `Status` objects
returned from functions. Using `StatusWith` to indicate exceptions, instead of throwing via
`uassert` and `internalAssert`, makes it very difficult to identify that an error has occurred, and
MongoDB provides `StatusWith` to enable functions to return an error code or a value without
requiring them to have multiple outputs. This makes exception-free code cleaner by avoiding
functions with multiple out parameters. We can either pass an error code or an actual value to a
`StatusWith` object, indicating failure or success of the operation. For examples of the proper
usage of `StatusWith`, see [mongo/base/status_with.h][status_with_h] and
[mongo/base/status_with_test.cpp][status_with_test_cpp]. It is highly recommended to use `uassert`
or `iassert` over `StatusWith`, and catch exceptions instead of checking `Status` objects
returned from functions. Using `StatusWith` to indicate exceptions, instead of throwing via
`uassert` and `iassert`, makes it very difficult to identify that an error has occurred, and
could lead to the wrong error being propagated.
## Gotchas

View File

@ -60,8 +60,8 @@ Nanoseconds getThreadCPUTime() {
struct timespec t;
if (auto ret = clock_gettime(CLOCK_THREAD_CPUTIME_ID, &t); ret != 0) {
int ec = errno;
internalAssert(Status(ErrorCodes::InternalError,
"Unable to get time: {}"_format(errnoWithDescription(ec))));
iassert(Status(ErrorCodes::InternalError,
"Unable to get time: {}"_format(errnoWithDescription(ec))));
}
return Seconds(t.tv_sec) + Nanoseconds(t.tv_nsec);
}

View File

@ -1702,7 +1702,7 @@ void ExecCommandDatabase::_handleFailure(Status status) {
if (ErrorCodes::isA<ErrorCategory::CloseConnectionError>(status.code())) {
// Rethrow the exception to the top to signal that the client connection should be closed.
internalAssert(status);
iassert(status);
}
}
@ -1852,7 +1852,7 @@ Future<DbResponse> receivedCommands(std::shared_ptr<HandleRequest::ExecutionCont
.onError([execContext](Status status) {
if (ErrorCodes::isConnectionFatalMessageParseError(status.code())) {
// If this error needs to fail the connection, propagate it out.
internalAssert(status);
iassert(status);
}
auto opCtx = execContext->getOpCtx();
@ -1870,7 +1870,7 @@ Future<DbResponse> receivedCommands(std::shared_ptr<HandleRequest::ExecutionCont
if (ErrorCodes::isA<ErrorCategory::CloseConnectionError>(status.code())) {
// Return the exception to the top to signal that the client connection should be
// closed.
internalAssert(status);
iassert(status);
}
})
.then([execContext] { return makeCommandResponse(std::move(execContext)); });

View File

@ -70,8 +70,7 @@ void WireSpec::reset(Specification spec) {
BSONObj oldSpec, newSpec;
{
stdx::lock_guard<Latch> lk(_mutex);
internalAssert(
ErrorCodes::NotYetInitialized, "WireSpec is not yet initialized", isInitialized());
iassert(ErrorCodes::NotYetInitialized, "WireSpec is not yet initialized", isInitialized());
oldSpec = specToBSON(*_spec.get());
_spec = std::make_shared<Specification>(std::move(spec));

View File

@ -92,8 +92,8 @@ public:
* for execution on the service executor. May throw if "scheduleTask" returns a non-okay status.
*/
void schedule(OutOfLineExecutor::Task func) override {
internalAssert(scheduleTask([task = std::move(func)]() mutable { task(Status::OK()); },
ScheduleFlags::kEmptyFlags));
iassert(scheduleTask([task = std::move(func)]() mutable { task(Status::OK()); },
ScheduleFlags::kEmptyFlags));
}
/*

View File

@ -160,7 +160,7 @@ Status ServiceExecutorFixed::scheduleTask(Task task, ScheduleFlags flags) {
// May throw if an attempt is made to schedule after the thread pool is shutdown.
try {
_threadPool->schedule([task = std::move(task)](Status status) mutable {
internalAssert(status);
iassert(status);
invariant(_executorContext);
_executorContext->run(std::move(task));
});

View File

@ -269,7 +269,7 @@ MONGO_COMPILER_NOINLINE void msgassertedWithLocation(const Status& status,
error_details::throwExceptionForStatus(status);
}
void internalAssertWithLocation(SourceLocationHolder loc, const Status& status) {
void iassertWithLocation(SourceLocationHolder loc, const Status& status) {
if (status.isOK())
return;
LOGV2_DEBUG(4892201, 3, "Internal assertion", "error"_attr = status, "location"_attr = loc);

View File

@ -437,38 +437,25 @@ inline void massertStatusOKWithLocation(const Status& status, const char* file,
}
/**
* `internalAssert` is provided as an alternative for `uassert` variants (e.g., `uassertStatusOK`)
* `iassert` is provided as an alternative for `uassert` variants (e.g., `uassertStatusOK`)
* to support cases where we expect a failure, the failure is recoverable, or accounting for the
* failure, updating assertion counters, isn't desired. `internalAssert` logs at D3 instead of D1,
* failure, updating assertion counters, isn't desired. `iassert` logs at D3 instead of D1,
* which helps with reducing the noise of assertions in production. The goal is to keep one
* interface (i.e., `internalAssert(...)`) for all possible assertion variants, and use function
* interface (i.e., `iassert(...)`) for all possible assertion variants, and use function
* overloading to expand type support as needed.
*/
#define internalAssert(...) \
::mongo::internalAssertWithLocation(MONGO_SOURCE_LOCATION(), __VA_ARGS__)
#define iassert(...) ::mongo::iassertWithLocation(MONGO_SOURCE_LOCATION(), __VA_ARGS__)
void internalAssertWithLocation(SourceLocationHolder loc, const Status& status);
void iassertWithLocation(SourceLocationHolder loc, const Status& status);
inline void internalAssertWithLocation(SourceLocationHolder loc, Status&& status) {
internalAssertWithLocation(std::move(loc), status);
}
inline void internalAssertWithLocation(SourceLocationHolder loc,
int msgid,
const std::string& msg,
bool expr) {
inline void iassertWithLocation(SourceLocationHolder loc, int msgid, std::string msg, bool expr) {
if (MONGO_unlikely(!expr))
internalAssertWithLocation(std::move(loc), Status(ErrorCodes::Error(msgid), msg));
iassertWithLocation(std::move(loc), Status(ErrorCodes::Error(msgid), std::move(msg)));
}
template <typename T>
inline void internalAssertWithLocation(SourceLocationHolder loc, const StatusWith<T>& sw) {
internalAssertWithLocation(std::move(loc), sw.getStatus());
}
template <typename T>
inline void internalAssertWithLocation(SourceLocationHolder loc, StatusWith<T>&& sw) {
internalAssertWithLocation(std::move(loc), sw);
void iassertWithLocation(SourceLocationHolder loc, const StatusWith<T>& sw) {
iassertWithLocation(std::move(loc), sw.getStatus());
}
/**

View File

@ -233,13 +233,13 @@ TEST(AssertUtils, InternalAssertWithStatus) {
auto userAssertions = assertionCount.user.load();
try {
Status status = {ErrorCodes::BadValue, "Test"};
internalAssert(status);
iassert(status);
} catch (const DBException& ex) {
ASSERT_EQ(ex.code(), ErrorCodes::BadValue);
ASSERT_EQ(ex.reason(), "Test");
}
internalAssert(Status::OK());
iassert(Status::OK());
ASSERT_EQ(userAssertions, assertionCount.user.load());
}
@ -247,13 +247,13 @@ TEST(AssertUtils, InternalAssertWithStatus) {
TEST(AssertUtils, InternalAssertWithExpression) {
auto userAssertions = assertionCount.user.load();
try {
internalAssert(48922, "Test", false);
iassert(48922, "Test", false);
} catch (const DBException& ex) {
ASSERT_EQ(ex.code(), 48922);
ASSERT_EQ(ex.reason(), "Test");
}
internalAssert(48922, "Another test", true);
iassert(48922, "Another test", true);
ASSERT_EQ(userAssertions, assertionCount.user.load());
}

View File

@ -317,7 +317,7 @@ public:
* Raises a AssertionException if this operation is in a killed state.
*/
void checkForInterrupt() {
internalAssert(checkForInterruptNoAssert());
iassert(checkForInterruptNoAssert());
}
/**
@ -395,7 +395,7 @@ public:
if (!swResult.isOK()) {
_onWake(latchName, WakeReason::kInterrupt, speed);
internalAssert(std::move(swResult));
iassert(std::move(swResult));
}
if (pred()) {