SERVER-31995 omit cloner stats from initial sync stats if too large

(cherry picked from commit 1a605a99bdc242fd957afdb6e9fe9b8f9c32c862)

3 files changed, 36 insertions, 20 deletions

diff --git a/src/mongo/db/repl/initial_syncer.cpp b/src/mongo/db/repl/initial_syncer.cpp
index 8349428c3fd..53f020541b0 100644
--- a/src/mongo/db/repl/initial_syncer.cpp
+++ b/src/mongo/db/repl/initial_syncer.cpp
@@ -365,30 +365,44 @@ BSONObj InitialSyncer::getInitialSyncProgress() const {
     return _getInitialSyncProgress_inlock();
 }
 
+void InitialSyncer::_appendInitialSyncProgressMinimal_inlock(BSONObjBuilder* bob) const {
+    _stats.append(bob);
+    if (!_initialSyncState) {
+        return;
+    }
+    bob->appendNumber("fetchedMissingDocs", _initialSyncState->fetchedMissingDocs);
+    bob->appendNumber("appliedOps", _initialSyncState->appliedOps);
+    if (!_initialSyncState->beginTimestamp.isNull()) {
+        bob->append("initialSyncOplogStart", _initialSyncState->beginTimestamp);
+    }
+    if (!_initialSyncState->stopTimestamp.isNull()) {
+        bob->append("initialSyncOplogEnd", _initialSyncState->stopTimestamp);
+    }
+}
+
 BSONObj InitialSyncer::_getInitialSyncProgress_inlock() const {
-    BSONObjBuilder bob;
     try {
-        _stats.append(&bob);
+        BSONObjBuilder bob;
+        _appendInitialSyncProgressMinimal_inlock(&bob);
         if (_initialSyncState) {
-            bob.appendNumber("fetchedMissingDocs", _initialSyncState->fetchedMissingDocs);
-            bob.appendNumber("appliedOps", _initialSyncState->appliedOps);
-            if (!_initialSyncState->beginTimestamp.isNull()) {
-                bob.append("initialSyncOplogStart", _initialSyncState->beginTimestamp);
-            }
-            if (!_initialSyncState->stopTimestamp.isNull()) {
-                bob.append("initialSyncOplogEnd", _initialSyncState->stopTimestamp);
-            }
             if (_initialSyncState->dbsCloner) {
                 BSONObjBuilder dbsBuilder(bob.subobjStart("databases"));
                 _initialSyncState->dbsCloner->getStats().append(&dbsBuilder);
                 dbsBuilder.doneFast();
             }
         }
+        // In 3.4, BSONObjBuilder::obj() does not check the validity of the document it returns so
+        // we check it explicitly here.
+        auto obj = bob.obj();
+        uassert(ErrorCodes::InvalidBSON,
+                str::stream() << "Invalid BSONObj size: " << obj.objsize(),
+                obj.isValid());
+        return obj;
     } catch (const DBException& e) {
-        bob.resetToEmpty();
-        bob.append("error", e.toString());
         log() << "Error creating initial sync progress object: " << e.toString();
     }
+    BSONObjBuilder bob;
+    _appendInitialSyncProgressMinimal_inlock(&bob);
     return bob.obj();
 }
 
diff --git a/src/mongo/db/repl/initial_syncer.h b/src/mongo/db/repl/initial_syncer.h
index 6faeddb385e..cf91932f062 100644
--- a/src/mongo/db/repl/initial_syncer.h
+++ b/src/mongo/db/repl/initial_syncer.h
@@ -35,6 +35,7 @@
 
 #include "mongo/base/status.h"
 #include "mongo/bson/bsonobj.h"
+#include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/bson/timestamp.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/repl/callback_completion_guard.h"
@@ -480,6 +481,7 @@ private:
                              Fetcher::Documents::const_iterator end,
                              const OplogFetcher::DocumentsInfo& info);
 
+    void _appendInitialSyncProgressMinimal_inlock(BSONObjBuilder* bob) const;
     BSONObj _getInitialSyncProgress_inlock() const;
 
     StatusWith<Operations> _getNextApplierBatch_inlock();
diff --git a/src/mongo/db/repl/initial_syncer_test.cpp b/src/mongo/db/repl/initial_syncer_test.cpp
index a50879bc432..3895f10fe3f 100644
--- a/src/mongo/db/repl/initial_syncer_test.cpp
+++ b/src/mongo/db/repl/initial_syncer_test.cpp
@@ -61,7 +61,6 @@
 #include "mongo/util/scopeguard.h"
 
 #include "mongo/unittest/barrier.h"
-#include "mongo/unittest/death_test.h"
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
@@ -3589,9 +3588,7 @@ TEST_F(InitialSyncerTest, GetInitialSyncProgressReturnsCorrectProgress) {
         << attempt1;
 }
 
-DEATH_TEST_F(InitialSyncerTest,
-             GetInitialSyncProgressThrowsExceptionIfClonerStatsExceedBsonLimit,
-             "terminate() called") {
+TEST_F(InitialSyncerTest, GetInitialSyncProgressOmitsClonerStatsIfClonerStatsExceedBsonLimit) {
     auto initialSyncer = &getInitialSyncer();
     auto opCtx = makeOpCtx();
 
@@ -3637,11 +3634,14 @@ DEATH_TEST_F(InitialSyncerTest,
         net->runReadyNetworkOperations();
     }
 
-    // This should throw because we are unable to fit all the cloner stats into a BSON document.
-    ASSERT_THROWS(initialSyncer->getInitialSyncProgress(), DBException);
+    // This returns a valid document because we omit the cloner stats when they do not fit in a
+    // BSON document.
+    auto progress = initialSyncer->getInitialSyncProgress();
+    ASSERT_EQUALS(progress["initialSyncStart"].type(), Date) << progress;
+    ASSERT_FALSE(progress.hasField("databases")) << progress;
 
-    // Initial sync will attempt to log stats again at shutdown in a callback, where it will
-    // terminate because of the unhandled exception.
+    // Initial sync will attempt to log stats again at shutdown in a callback, where it should not
+    // terminate because we now return a valid stats document.
     ASSERT_OK(initialSyncer->shutdown());
 
     // Deliver cancellation signal to callbacks.