diff options
author | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-10-17 15:35:49 -0400 |
---|---|---|
committer | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-10-18 16:38:34 -0400 |
commit | ddacafb22591d831f2b320b196c2809193a52d05 (patch) | |
tree | 0c1e803e0efb5c20717831dc4c363744b8b08f26 | |
parent | 18d5b0cea7558f88bbd5dcbec2a762b51cb13c98 (diff) | |
download | mongo-ddacafb22591d831f2b320b196c2809193a52d05.tar.gz |
SERVER-31531 featureCompatibilityVersion writes must check for writeErrors
-rw-r--r-- | jstests/multiVersion/set_feature_compatibility_version.js | 28 | ||||
-rw-r--r-- | src/mongo/db/commands/feature_compatibility_version.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.cpp | 17 | ||||
-rw-r--r-- | src/mongo/rpc/get_status_from_command_result.cpp | 43 | ||||
-rw-r--r-- | src/mongo/rpc/get_status_from_command_result.h | 13 |
5 files changed, 102 insertions, 2 deletions
diff --git a/jstests/multiVersion/set_feature_compatibility_version.js b/jstests/multiVersion/set_feature_compatibility_version.js index e7f1852ccd4..6b57097de9f 100644 --- a/jstests/multiVersion/set_feature_compatibility_version.js +++ b/jstests/multiVersion/set_feature_compatibility_version.js @@ -141,10 +141,38 @@ TestData.skipCheckingUUIDsConsistentAcrossCluster = true; // featureCompatibilityVersion cannot be set via setParameter. assert.commandFailed(adminDB.runCommand({setParameter: 1, featureCompatibilityVersion: "3.4"})); + // setFeatureCompatibilityVersion fails to downgrade to FCV=3.4 if the write fails. + assert.commandWorked(adminDB.runCommand({ + configureFailPoint: "failCollectionUpdates", + data: {collectionNS: "admin.system.version"}, + mode: "alwaysOn" + })); + assert.commandFailed(adminDB.runCommand({setFeatureCompatibilityVersion: "3.4"})); + checkFCV(adminDB, "3.6"); + assert.commandWorked(adminDB.runCommand({ + configureFailPoint: "failCollectionUpdates", + data: {collectionNS: "admin.system.version"}, + mode: "off" + })); + // featureCompatibilityVersion can be set to 3.4. assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: "3.4"})); checkFCV(adminDB, "3.4"); + // setFeatureCompatibilityVersion fails to upgrade to FCV=3.6 if the write fails. + assert.commandWorked(adminDB.runCommand({ + configureFailPoint: "failCollectionUpdates", + data: {collectionNS: "admin.system.version"}, + mode: "alwaysOn" + })); + assert.commandFailed(adminDB.runCommand({setFeatureCompatibilityVersion: "3.6"})); + checkFCV(adminDB, "3.4"); + assert.commandWorked(adminDB.runCommand({ + configureFailPoint: "failCollectionUpdates", + data: {collectionNS: "admin.system.version"}, + mode: "off" + })); + // featureCompatibilityVersion can be set to 3.6. assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: "3.6"})); checkFCV(adminDB, "3.6"); diff --git a/src/mongo/db/commands/feature_compatibility_version.cpp b/src/mongo/db/commands/feature_compatibility_version.cpp index 74f71843e84..c29e570cc0a 100644 --- a/src/mongo/db/commands/feature_compatibility_version.cpp +++ b/src/mongo/db/commands/feature_compatibility_version.cpp @@ -343,8 +343,7 @@ void FeatureCompatibilityVersion::_runUpdateCommand(OperationContext* opCtx, // collection. BSONObj updateResult; client.runCommand(nss.db().toString(), updateCmd.obj(), updateResult); - uassertStatusOK(getStatusFromCommandResult(updateResult)); - uassertStatusOK(getWriteConcernStatusFromCommandResult(updateResult)); + uassertStatusOK(getStatusFromWriteCommandReply(updateResult)); } /** * Read-only server parameter for featureCompatibilityVersion. diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index 559b44bf72d..0f3c08b0e07 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -51,10 +51,13 @@ #include "mongo/db/views/durable_view_catalog.h" #include "mongo/scripting/engine.h" #include "mongo/util/assert_util.h" +#include "mongo/util/fail_point_service.h" namespace mongo { namespace { +MONGO_FP_DECLARE(failCollectionUpdates); + /** * Returns whether we're a master using master-slave replication. */ @@ -317,6 +320,20 @@ void OpObserverImpl::onInserts(OperationContext* opCtx, } void OpObserverImpl::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArgs& args) { + MONGO_FAIL_POINT_BLOCK(failCollectionUpdates, extraData) { + auto collElem = extraData.getData()["collectionNS"]; + // If the failpoint specifies no collection or matches the existing one, fail. + if (!collElem || args.nss.ns() == collElem.String()) { + uasserted(40654, + str::stream() << "failCollectionUpdates failpoint enabled, namespace: " + << args.nss.ns() + << ", update: " + << args.update + << " on document with " + << args.criteria); + } + } + // Do not log a no-op operation; see SERVER-21738 if (args.update.isEmpty()) { return; diff --git a/src/mongo/rpc/get_status_from_command_result.cpp b/src/mongo/rpc/get_status_from_command_result.cpp index 2b4adf94aae..13c176f44df 100644 --- a/src/mongo/rpc/get_status_from_command_result.cpp +++ b/src/mongo/rpc/get_status_from_command_result.cpp @@ -30,6 +30,7 @@ #include "mongo/rpc/get_status_from_command_result.h" +#include "mongo/base/error_codes.h" #include "mongo/base/status.h" #include "mongo/bson/util/bson_extract.h" #include "mongo/db/jsobj.h" @@ -40,6 +41,7 @@ namespace mongo { namespace { const std::string kCmdResponseWriteConcernField = "writeConcernError"; +const std::string kCmdResponseWriteErrorsField = "writeErrors"; } // namespace Status getStatusFromCommandResult(const BSONObj& result) { @@ -107,4 +109,45 @@ Status getWriteConcernStatusFromCommandResult(const BSONObj& obj) { return wcError.toStatus(); } +Status getFirstWriteErrorStatusFromCommandResult(const BSONObj& cmdResponse) { + BSONElement writeErrorElem; + auto status = bsonExtractTypedField( + cmdResponse, kCmdResponseWriteErrorsField, BSONType::Array, &writeErrorElem); + if (!status.isOK()) { + if (status == ErrorCodes::NoSuchKey) { + return Status::OK(); + } else { + return status; + } + } + + auto firstWriteErrorElem = writeErrorElem.Obj().firstElement(); + if (!firstWriteErrorElem) { + return Status::OK(); + } + + if (firstWriteErrorElem.type() != BSONType::Object) { + return Status(ErrorCodes::UnsupportedFormat, + str::stream() << "writeErrors should be an array of objects, found " + << typeName(firstWriteErrorElem.type())); + } + + auto firstWriteErrorObj = firstWriteErrorElem.Obj(); + + return Status(ErrorCodes::fromInt(firstWriteErrorObj["code"].Int()), + firstWriteErrorObj["errmsg"].String()); +} + +Status getStatusFromWriteCommandReply(const BSONObj& cmdResponse) { + auto status = getStatusFromCommandResult(cmdResponse); + if (!status.isOK()) { + return status; + } + status = getFirstWriteErrorStatusFromCommandResult(cmdResponse); + if (!status.isOK()) { + return status; + } + return getWriteConcernStatusFromCommandResult(cmdResponse); +} + } // namespace mongo diff --git a/src/mongo/rpc/get_status_from_command_result.h b/src/mongo/rpc/get_status_from_command_result.h index 971805ee088..423b656a0ed 100644 --- a/src/mongo/rpc/get_status_from_command_result.h +++ b/src/mongo/rpc/get_status_from_command_result.h @@ -51,4 +51,17 @@ Status getStatusFromCommandResult(const BSONObj& result); */ Status getWriteConcernStatusFromCommandResult(const BSONObj& cmdResponse); + +/** + * Extracts the first write error from a command response and converts it into a status. This + * ignores all errors after the first and does not preserve the write error index, so it should not + * be used with bulk writes. + */ +Status getFirstWriteErrorStatusFromCommandResult(const BSONObj& cmdResponse); + +/** + * Extracts any type of error from a write command response. + */ +Status getStatusFromWriteCommandReply(const BSONObj& cmdResponse); + } // namespace mongo |