SERVER-39823 Free Monitoring may ignore re-register requests on secondaries

(cherry picked from commit df3f76da935a45302d7b24b0abe618af6dd34744)

6 files changed, 29 insertions, 16 deletions

diff --git a/jstests/free_mon/free_mon_rs_delete.js b/jstests/free_mon/free_mon_rs_delete.js
index 335d93cfef4..0995d05c3f8 100644
--- a/jstests/free_mon/free_mon_rs_delete.js
+++ b/jstests/free_mon/free_mon_rs_delete.js
@@ -24,8 +24,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
+    WaitForRegistration(rst.getPrimary());
+    WaitForRegistration(rst.getSecondary());
 
     const qs1 = mock_web.queryStats();
 
@@ -52,8 +52,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     sleep(20 * 1000);
 
-    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
+    WaitForRegistration(rst.getPrimary());
+    WaitForRegistration(rst.getSecondary());
 
     rst.stopSet();
 
diff --git a/jstests/free_mon/free_mon_rs_perm_del.js b/jstests/free_mon/free_mon_rs_perm_del.js
index 3f15719062d..b8cb3e73a59 100644
--- a/jstests/free_mon/free_mon_rs_perm_del.js
+++ b/jstests/free_mon/free_mon_rs_perm_del.js
@@ -24,8 +24,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
+    WaitForRegistration(rst.getPrimary());
+    WaitForRegistration(rst.getSecondary());
 
     mock_web.enableFaults();
     mock_web.waitFaults(1);
diff --git a/jstests/free_mon/free_mon_rs_register.js b/jstests/free_mon/free_mon_rs_register.js
index acbdab3301c..cabb82077fb 100644
--- a/jstests/free_mon/free_mon_rs_register.js
+++ b/jstests/free_mon/free_mon_rs_register.js
@@ -28,8 +28,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
+    WaitForRegistration(rst.getPrimary());
+    WaitForRegistration(rst.getSecondary());
 
     const last_register = mock_web.query("last_register");
     print(tojson(last_register));
diff --git a/src/mongo/db/free_mon/free_mon_controller_test.cpp b/src/mongo/db/free_mon/free_mon_controller_test.cpp
index e1d8e3610ac..aa5aab2d9bf 100644
--- a/src/mongo/db/free_mon/free_mon_controller_test.cpp
+++ b/src/mongo/db/free_mon/free_mon_controller_test.cpp
@@ -1462,7 +1462,7 @@ TEST_F(FreeMonControllerRSTest, StepdownDuringRegistration) {
     ASSERT_TRUE(FreeMonStorage::read(_opCtx.get()).get().getState() == StorageStateEnum::pending);
 
     ASSERT_EQ(controller.registerCollector->count(), 1UL);
-    ASSERT_EQ(controller.metricsCollector->count(), 3UL);
+    ASSERT_EQ(controller.metricsCollector->count(), 2UL);
 }
 
 // Negative: Tricky: Primary becomes secondary during metrics send
diff --git a/src/mongo/db/free_mon/free_mon_processor.cpp b/src/mongo/db/free_mon/free_mon_processor.cpp
index e12c8c07efb..e13b93d11fd 100644
--- a/src/mongo/db/free_mon/free_mon_processor.cpp
+++ b/src/mongo/db/free_mon/free_mon_processor.cpp
@@ -266,7 +266,7 @@ void FreeMonProcessor::run() {
     }
 }
 
-void FreeMonProcessor::readState(OperationContext* opCtx) {
+void FreeMonProcessor::readState(OperationContext* opCtx, bool updateInMemory) {
     auto state = FreeMonStorage::read(opCtx);
 
     _lastReadState = state;
@@ -274,7 +274,9 @@ void FreeMonProcessor::readState(OperationContext* opCtx) {
     if (state.is_initialized()) {
         invariant(state.get().getVersion() == kStorageVersion);
 
-        _state = state.get();
+        if (updateInMemory) {
+            _state = state.get();
+        }
     } else if (!state.is_initialized()) {
         // Default the state
         auto state = _state.synchronize();
@@ -287,9 +289,9 @@ void FreeMonProcessor::readState(OperationContext* opCtx) {
     }
 }
 
-void FreeMonProcessor::readState(Client* client) {
+void FreeMonProcessor::readState(Client* client, bool updateInMemory) {
     auto opCtx = client->makeOperationContext();
-    readState(opCtx.get());
+    readState(opCtx.get(), updateInMemory);
 }
 
 void FreeMonProcessor::writeState(Client* client) {
@@ -743,7 +745,10 @@ std::string compressMetrics(MetricsBuffer& buffer) {
 }
 
 void FreeMonProcessor::doMetricsSend(Client* client) {
-    readState(client);
+    // We want to read state from disk in case we asked to stop but otherwise
+    // use the in-memory state. It is important not to treat disk state as authoritative
+    // on secondaries.
+    readState(client, false);
 
     // Only continue metrics send if the local disk state (in-case user deleted local document)
     // and in-memory status both say to continue.
diff --git a/src/mongo/db/free_mon/free_mon_processor.h b/src/mongo/db/free_mon/free_mon_processor.h
index 129143cf46a..312ed981874 100644
--- a/src/mongo/db/free_mon/free_mon_processor.h
+++ b/src/mongo/db/free_mon/free_mon_processor.h
@@ -342,13 +342,21 @@ public:
 private:
     /**
      * Read the state from the database.
+     *
+     * Checks if the storage document has been delete locally or does not exist. If it is missing,
+     * generates a default disable state.
+     *
+     * If updateInMemory is true, update the state in memory with the state from disk. If false, do
+     * not update the state in memory from disk but instead treat the state in memory as
+     * authoritative. The is important for secondaries which may be in a different state for
+     * regsistration then there primary.
      */
-    void readState(OperationContext* opCtx);
+    void readState(OperationContext* opCtx, bool updateInMemory = true);
 
     /**
      * Create a short-lived opCtx and read the state from the database.
      */
-    void readState(Client* client);
+    void readState(Client* client, bool updateInMemory = true);
 
     /**
      * Write the state to disk if there are any changes.