SERVER-40105 Improve diagnostic information in currentOp for writeConflicts and prepareConflicts

4 files changed, 110 insertions, 49 deletions

diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index 2111f168edf..cf6b63648e6 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -575,6 +575,13 @@ void CurOp::reportState(BSONObjBuilder* builder, bool truncateOps) {
         }
     }
 
+    if (auto n = _debug.additiveMetrics.prepareReadConflicts.load(); n > 0) {
+        builder->append("prepareReadConflicts", n);
+    }
+    if (auto n = _debug.additiveMetrics.writeConflicts.load(); n > 0) {
+        builder->append("writeConflicts", n);
+    }
+
     builder->append("numYields", _numYields);
 }
 
@@ -595,6 +602,9 @@ StringData getProtoString(int op) {
 #define OPDEBUG_TOSTRING_HELP_BOOL(x) \
     if (x)                            \
     s << " " #x ":" << (x)
+#define OPDEBUG_TOSTRING_HELP_ATOMIC(x, y) \
+    if (auto __y = y.load(); __y > 0)      \
+    s << " " x ":" << (__y)
 #define OPDEBUG_TOSTRING_HELP_OPTIONAL(x, y) \
     if (y)                                   \
     s << " " x ":" << (*y)
@@ -672,8 +682,8 @@ string OpDebug::report(Client* client,
 
     OPDEBUG_TOSTRING_HELP_OPTIONAL("keysInserted", additiveMetrics.keysInserted);
     OPDEBUG_TOSTRING_HELP_OPTIONAL("keysDeleted", additiveMetrics.keysDeleted);
-    OPDEBUG_TOSTRING_HELP_OPTIONAL("prepareReadConflicts", additiveMetrics.prepareReadConflicts);
-    OPDEBUG_TOSTRING_HELP_OPTIONAL("writeConflicts", additiveMetrics.writeConflicts);
+    OPDEBUG_TOSTRING_HELP_ATOMIC("prepareReadConflicts", additiveMetrics.prepareReadConflicts);
+    OPDEBUG_TOSTRING_HELP_ATOMIC("writeConflicts", additiveMetrics.writeConflicts);
 
     s << " numYields:" << curop.numYields();
     OPDEBUG_TOSTRING_HELP(nreturned);
@@ -727,6 +737,9 @@ string OpDebug::report(Client* client,
 #define OPDEBUG_APPEND_BOOL(x) \
     if (x)                     \
     b.appendBool(#x, (x))
+#define OPDEBUG_APPEND_ATOMIC(x, y)   \
+    if (auto __y = y.load(); __y > 0) \
+    b.appendNumber(x, __y)
 #define OPDEBUG_APPEND_OPTIONAL(x, y) \
     if (y)                            \
     b.appendNumber(x, (*y))
@@ -769,8 +782,8 @@ void OpDebug::append(const CurOp& curop,
 
     OPDEBUG_APPEND_OPTIONAL("keysInserted", additiveMetrics.keysInserted);
     OPDEBUG_APPEND_OPTIONAL("keysDeleted", additiveMetrics.keysDeleted);
-    OPDEBUG_APPEND_OPTIONAL("prepareReadConflicts", additiveMetrics.prepareReadConflicts);
-    OPDEBUG_APPEND_OPTIONAL("writeConflicts", additiveMetrics.writeConflicts);
+    OPDEBUG_APPEND_ATOMIC("prepareReadConflicts", additiveMetrics.prepareReadConflicts);
+    OPDEBUG_APPEND_ATOMIC("writeConflicts", additiveMetrics.writeConflicts);
 
     b.appendNumber("numYield", curop.numYields());
     OPDEBUG_APPEND_NUMBER(nreturned);
@@ -872,25 +885,34 @@ void OpDebug::AdditiveMetrics::add(const AdditiveMetrics& otherMetrics) {
     ndeleted = addOptionalLongs(ndeleted, otherMetrics.ndeleted);
     keysInserted = addOptionalLongs(keysInserted, otherMetrics.keysInserted);
     keysDeleted = addOptionalLongs(keysDeleted, otherMetrics.keysDeleted);
-    prepareReadConflicts =
-        addOptionalLongs(prepareReadConflicts, otherMetrics.prepareReadConflicts);
-    writeConflicts = addOptionalLongs(writeConflicts, otherMetrics.writeConflicts);
+    prepareReadConflicts.fetchAndAdd(otherMetrics.prepareReadConflicts.load());
+    writeConflicts.fetchAndAdd(otherMetrics.writeConflicts.load());
 }
 
-bool OpDebug::AdditiveMetrics::equals(const AdditiveMetrics& otherMetrics) {
+void OpDebug::AdditiveMetrics::reset() {
+    keysExamined = boost::none;
+    docsExamined = boost::none;
+    nMatched = boost::none;
+    nModified = boost::none;
+    ninserted = boost::none;
+    ndeleted = boost::none;
+    keysInserted = boost::none;
+    keysDeleted = boost::none;
+    prepareReadConflicts.store(0);
+    writeConflicts.store(0);
+}
+
+bool OpDebug::AdditiveMetrics::equals(const AdditiveMetrics& otherMetrics) const {
     return keysExamined == otherMetrics.keysExamined && docsExamined == otherMetrics.docsExamined &&
         nMatched == otherMetrics.nMatched && nModified == otherMetrics.nModified &&
         ninserted == otherMetrics.ninserted && ndeleted == otherMetrics.ndeleted &&
         keysInserted == otherMetrics.keysInserted && keysDeleted == otherMetrics.keysDeleted &&
-        prepareReadConflicts == otherMetrics.prepareReadConflicts &&
-        writeConflicts == otherMetrics.writeConflicts;
+        prepareReadConflicts.load() == otherMetrics.prepareReadConflicts.load() &&
+        writeConflicts.load() == otherMetrics.writeConflicts.load();
 }
 
 void OpDebug::AdditiveMetrics::incrementWriteConflicts(long long n) {
-    if (!writeConflicts) {
-        writeConflicts = 0;
-    }
-    *writeConflicts += n;
+    writeConflicts.fetchAndAdd(n);
 }
 
 void OpDebug::AdditiveMetrics::incrementKeysInserted(long long n) {
@@ -915,10 +937,7 @@ void OpDebug::AdditiveMetrics::incrementNinserted(long long n) {
 }
 
 void OpDebug::AdditiveMetrics::incrementPrepareReadConflicts(long long n) {
-    if (!prepareReadConflicts) {
-        prepareReadConflicts = 0;
-    }
-    *prepareReadConflicts += n;
+    prepareReadConflicts.fetchAndAdd(n);
 }
 
 string OpDebug::AdditiveMetrics::report() const {
@@ -932,8 +951,8 @@ string OpDebug::AdditiveMetrics::report() const {
     OPDEBUG_TOSTRING_HELP_OPTIONAL("ndeleted", ndeleted);
     OPDEBUG_TOSTRING_HELP_OPTIONAL("keysInserted", keysInserted);
     OPDEBUG_TOSTRING_HELP_OPTIONAL("keysDeleted", keysDeleted);
-    OPDEBUG_TOSTRING_HELP_OPTIONAL("prepareReadConflicts", prepareReadConflicts);
-    OPDEBUG_TOSTRING_HELP_OPTIONAL("writeConflicts", writeConflicts);
+    OPDEBUG_TOSTRING_HELP_ATOMIC("prepareReadConflicts", prepareReadConflicts);
+    OPDEBUG_TOSTRING_HELP_ATOMIC("writeConflicts", writeConflicts);
 
     return s.str();
 }
diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h
index a2466ffeeb2..ba1438f92a5 100644
--- a/src/mongo/db/curop.h
+++ b/src/mongo/db/curop.h
@@ -55,6 +55,17 @@ public:
      */
     class AdditiveMetrics {
     public:
+        AdditiveMetrics() = default;
+        AdditiveMetrics(const AdditiveMetrics& other) {
+            this->add(other);
+        }
+
+        AdditiveMetrics& operator=(const AdditiveMetrics& other) {
+            reset();
+            add(other);
+            return *this;
+        }
+
         /**
          * Adds all the fields of another AdditiveMetrics object together with the fields of this
          * AdditiveMetrics instance.
@@ -62,10 +73,15 @@ public:
         void add(const AdditiveMetrics& otherMetrics);
 
         /**
+         * Resets all members to the default state.
+         */
+        void reset();
+
+        /**
          * Returns true if the AdditiveMetrics object we are comparing has the same field values as
          * this AdditiveMetrics instance.
          */
-        bool equals(const AdditiveMetrics& otherMetrics);
+        bool equals(const AdditiveMetrics& otherMetrics) const;
 
         /**
          * Increments writeConflicts by n.
@@ -113,9 +129,15 @@ public:
         boost::optional<long long> keysInserted;
         // Number of index keys removed.
         boost::optional<long long> keysDeleted;
+
+        // The following fields are atomic because they are reported by CurrentOp. This is an
+        // exception to the prescription that OpDebug only be used by the owning thread because
+        // these metrics are tracked over the course of a transaction by SingleTransactionStats,
+        // which is built on OpDebug.
+
         // Number of read conflicts caused by a prepared transaction.
-        boost::optional<long long> prepareReadConflicts;
-        boost::optional<long long> writeConflicts;
+        AtomicWord<long long> prepareReadConflicts{0};
+        AtomicWord<long long> writeConflicts{0};
     };
 
     OpDebug() = default;
diff --git a/src/mongo/db/curop_metrics.cpp b/src/mongo/db/curop_metrics.cpp
index a625a9bd2f1..0bc9a23f387 100644
--- a/src/mongo/db/curop_metrics.cpp
+++ b/src/mongo/db/curop_metrics.cpp
@@ -78,8 +78,8 @@ void recordCurOpMetrics(OperationContext* opCtx) {
 
     if (debug.hasSortStage)
         scanAndOrderCounter.increment();
-    if (debug.additiveMetrics.writeConflicts)
-        writeConflictsCounter.increment(*debug.additiveMetrics.writeConflicts);
+    if (auto n = debug.additiveMetrics.writeConflicts.load(); n > 0)
+        writeConflictsCounter.increment(n);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/curop_test.cpp b/src/mongo/db/curop_test.cpp
index b472c71e4de..abda927f3c8 100644
--- a/src/mongo/db/curop_test.cpp
+++ b/src/mongo/db/curop_test.cpp
@@ -39,6 +39,22 @@
 namespace mongo {
 namespace {
 
+TEST(CurOpTest, CopyConstructors) {
+    OpDebug::AdditiveMetrics a, b;
+    a.keysExamined = 1;
+    b.keysExamined = 2;
+    a.prepareReadConflicts.store(1);
+    b.prepareReadConflicts.store(2);
+    // Test copy constructor.
+    OpDebug::AdditiveMetrics c = a;
+    ASSERT_EQ(a.keysExamined, c.keysExamined);
+    ASSERT_EQ(a.prepareReadConflicts.load(), c.prepareReadConflicts.load());
+    // Test copy assignment.
+    a = b;
+    ASSERT_EQ(a.keysExamined, b.keysExamined);
+    ASSERT_EQ(a.prepareReadConflicts.load(), b.prepareReadConflicts.load());
+}
+
 TEST(CurOpTest, AddingAdditiveMetricsObjectsTogetherShouldAddFieldsTogether) {
     OpDebug::AdditiveMetrics currentAdditiveMetrics = OpDebug::AdditiveMetrics();
     OpDebug::AdditiveMetrics additiveMetricsToAdd = OpDebug::AdditiveMetrics();
@@ -60,13 +76,14 @@ TEST(CurOpTest, AddingAdditiveMetricsObjectsTogetherShouldAddFieldsTogether) {
     additiveMetricsToAdd.keysInserted = 5;
     currentAdditiveMetrics.keysDeleted = 4;
     additiveMetricsToAdd.keysDeleted = 2;
-    currentAdditiveMetrics.prepareReadConflicts = 1;
-    additiveMetricsToAdd.prepareReadConflicts = 5;
-    currentAdditiveMetrics.writeConflicts = 7;
-    additiveMetricsToAdd.writeConflicts = 0;
+    currentAdditiveMetrics.prepareReadConflicts.store(1);
+    additiveMetricsToAdd.prepareReadConflicts.store(5);
+    currentAdditiveMetrics.writeConflicts.store(7);
+    additiveMetricsToAdd.writeConflicts.store(0);
 
     // Save the current AdditiveMetrics object before adding.
-    OpDebug::AdditiveMetrics additiveMetricsBeforeAdd = currentAdditiveMetrics;
+    OpDebug::AdditiveMetrics additiveMetricsBeforeAdd;
+    additiveMetricsBeforeAdd.add(currentAdditiveMetrics);
     currentAdditiveMetrics.add(additiveMetricsToAdd);
 
     // The following field values should have changed after adding.
@@ -86,11 +103,12 @@ TEST(CurOpTest, AddingAdditiveMetricsObjectsTogetherShouldAddFieldsTogether) {
               *additiveMetricsBeforeAdd.keysInserted + *additiveMetricsToAdd.keysInserted);
     ASSERT_EQ(*currentAdditiveMetrics.keysDeleted,
               *additiveMetricsBeforeAdd.keysDeleted + *additiveMetricsToAdd.keysDeleted);
-    ASSERT_EQ(*currentAdditiveMetrics.prepareReadConflicts,
-              *additiveMetricsBeforeAdd.prepareReadConflicts +
-                  *additiveMetricsToAdd.prepareReadConflicts);
-    ASSERT_EQ(*currentAdditiveMetrics.writeConflicts,
-              *additiveMetricsBeforeAdd.writeConflicts + *additiveMetricsToAdd.writeConflicts);
+    ASSERT_EQ(currentAdditiveMetrics.prepareReadConflicts.load(),
+              additiveMetricsBeforeAdd.prepareReadConflicts.load() +
+                  additiveMetricsToAdd.prepareReadConflicts.load());
+    ASSERT_EQ(currentAdditiveMetrics.writeConflicts.load(),
+              additiveMetricsBeforeAdd.writeConflicts.load() +
+                  additiveMetricsToAdd.writeConflicts.load());
 }
 
 TEST(CurOpTest, AddingUninitializedAdditiveMetricsFieldsShouldBeTreatedAsZero) {
@@ -106,13 +124,14 @@ TEST(CurOpTest, AddingUninitializedAdditiveMetricsFieldsShouldBeTreatedAsZero) {
     additiveMetricsToAdd.keysInserted = 5;
     currentAdditiveMetrics.keysDeleted = 4;
     additiveMetricsToAdd.keysDeleted = 2;
-    currentAdditiveMetrics.prepareReadConflicts = 1;
-    additiveMetricsToAdd.prepareReadConflicts = 5;
-    currentAdditiveMetrics.writeConflicts = 7;
-    additiveMetricsToAdd.writeConflicts = 0;
+    currentAdditiveMetrics.prepareReadConflicts.store(1);
+    additiveMetricsToAdd.prepareReadConflicts.store(5);
+    currentAdditiveMetrics.writeConflicts.store(7);
+    additiveMetricsToAdd.writeConflicts.store(0);
 
     // Save the current AdditiveMetrics object before adding.
-    OpDebug::AdditiveMetrics additiveMetricsBeforeAdd = currentAdditiveMetrics;
+    OpDebug::AdditiveMetrics additiveMetricsBeforeAdd;
+    additiveMetricsBeforeAdd.add(currentAdditiveMetrics);
     currentAdditiveMetrics.add(additiveMetricsToAdd);
 
     // The 'keysExamined' field for the current AdditiveMetrics object was not initialized, so it
@@ -132,20 +151,21 @@ TEST(CurOpTest, AddingUninitializedAdditiveMetricsFieldsShouldBeTreatedAsZero) {
               *additiveMetricsBeforeAdd.keysInserted + *additiveMetricsToAdd.keysInserted);
     ASSERT_EQ(*currentAdditiveMetrics.keysDeleted,
               *additiveMetricsBeforeAdd.keysDeleted + *additiveMetricsToAdd.keysDeleted);
-    ASSERT_EQ(*currentAdditiveMetrics.prepareReadConflicts,
-              *additiveMetricsBeforeAdd.prepareReadConflicts +
-                  *additiveMetricsToAdd.prepareReadConflicts);
-    ASSERT_EQ(*currentAdditiveMetrics.writeConflicts,
-              *additiveMetricsBeforeAdd.writeConflicts + *additiveMetricsToAdd.writeConflicts);
+    ASSERT_EQ(currentAdditiveMetrics.prepareReadConflicts.load(),
+              additiveMetricsBeforeAdd.prepareReadConflicts.load() +
+                  additiveMetricsToAdd.prepareReadConflicts.load());
+    ASSERT_EQ(currentAdditiveMetrics.writeConflicts.load(),
+              additiveMetricsBeforeAdd.writeConflicts.load() +
+                  additiveMetricsToAdd.writeConflicts.load());
 }
 
 TEST(CurOpTest, AdditiveMetricsFieldsShouldIncrementByN) {
     OpDebug::AdditiveMetrics additiveMetrics = OpDebug::AdditiveMetrics();
 
     // Initialize field values.
-    additiveMetrics.writeConflicts = 1;
+    additiveMetrics.writeConflicts.store(1);
     additiveMetrics.keysInserted = 2;
-    additiveMetrics.prepareReadConflicts = 6;
+    additiveMetrics.prepareReadConflicts.store(6);
 
     // Increment the fields.
     additiveMetrics.incrementWriteConflicts(1);
@@ -154,11 +174,11 @@ TEST(CurOpTest, AdditiveMetricsFieldsShouldIncrementByN) {
     additiveMetrics.incrementNinserted(3);
     additiveMetrics.incrementPrepareReadConflicts(2);
 
-    ASSERT_EQ(*additiveMetrics.writeConflicts, 2);
+    ASSERT_EQ(additiveMetrics.writeConflicts.load(), 2);
     ASSERT_EQ(*additiveMetrics.keysInserted, 7);
     ASSERT_EQ(*additiveMetrics.keysDeleted, 0);
     ASSERT_EQ(*additiveMetrics.ninserted, 3);
-    ASSERT_EQ(*additiveMetrics.prepareReadConflicts, 8);
+    ASSERT_EQ(additiveMetrics.prepareReadConflicts.load(), 8);
 }
 
 TEST(CurOpTest, OptionalAdditiveMetricsNotDisplayedIfUninitialized) {