SERVER-31531 featureCompatibilityVersion writes must check for writeErrors

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