Fix wrong use of hello command in server monitoring

Author: alcaeus

src/libmongoc/src/mongoc/mongoc-topology-scanner-private.h

@@ -54,6 +54,7 @@ typedef struct mongoc_topology_scanner_node {
    int64_t last_used;
    int64_t last_failed;
    bool has_auth;
+   bool hello_ok;
    mongoc_host_list_t host;
    struct mongoc_topology_scanner *ts;
 
@@ -131,7 +132,8 @@ mongoc_topology_scanner_valid (mongoc_topology_scanner_t *ts);
 void
 mongoc_topology_scanner_add (mongoc_topology_scanner_t *ts,
                              const mongoc_host_list_t *host,
-                             uint32_t id);
+                             uint32_t id,
+                             bool hello_ok);
 
 void
 mongoc_topology_scanner_scan (mongoc_topology_scanner_t *ts, uint32_t id);

src/libmongoc/src/mongoc/mongoc-topology-scanner.c

@@ -327,7 +327,8 @@ _begin_hello_cmd (mongoc_topology_scanner_node_t *node,
 
    if (node->last_used != -1 && node->last_failed == -1) {
       /* The node's been used before and not failed recently */
-      bson_copy_to (&ts->hello_cmd, &cmd);
+      bson_copy_to (node->hello_ok ? &ts->hello_cmd : &ts->legacy_hello_cmd,
+                    &cmd);
    } else {
       bson_copy_to (_mongoc_topology_scanner_get_handshake_cmd (ts), &cmd);
    }
@@ -457,7 +458,8 @@ mongoc_topology_scanner_valid (mongoc_topology_scanner_t *ts)
 void
 mongoc_topology_scanner_add (mongoc_topology_scanner_t *ts,
                              const mongoc_host_list_t *host,
-                             uint32_t id)
+                             uint32_t id,
+                             bool hello_ok)
 {
    mongoc_topology_scanner_node_t *node;
 
@@ -469,6 +471,7 @@ mongoc_topology_scanner_add (mongoc_topology_scanner_t *ts,
    node->ts = ts;
    node->last_failed = -1;
    node->last_used = -1;
+   node->hello_ok = hello_ok;
    bson_init (&node->speculative_auth_response);
 
    DL_APPEND (ts->nodes, node);

src/libmongoc/src/mongoc/mongoc-topology.c

@@ -43,12 +43,19 @@ _mongoc_topology_reconcile_add_nodes (mongoc_server_description_t *sd,
                                       mongoc_topology_t *topology)
 {
    mongoc_topology_scanner_t *scanner = topology->scanner;
+   mongoc_topology_scanner_node_t *node;
+
+   /* Search by ID and update hello_ok */
+   node = mongoc_topology_scanner_get_node (scanner, sd->id);
+   if (node) {
+      node->hello_ok = sd->hello_ok;
+
+      return true;
+   }
 
-   /* quickly search by id, then check if a node for this host was retired in
-    * this scan. */
-   if (!mongoc_topology_scanner_get_node (scanner, sd->id) &&
-       !mongoc_topology_scanner_has_node_for_host (scanner, &sd->host)) {
-      mongoc_topology_scanner_add (scanner, &sd->host, sd->id);
+   /* Check if a node for this host was retired in this scan. */
+   if (!mongoc_topology_scanner_has_node_for_host (scanner, &sd->host)) {
+      mongoc_topology_scanner_add (scanner, &sd->host, sd->id, sd->hello_ok);
       mongoc_topology_scanner_scan (scanner, sd->id);
    }
 
@@ -189,9 +196,9 @@ _mongoc_topology_scanner_cb (uint32_t id,
       _mongoc_topology_update_no_lock (
          id, hello_response, rtt_msec, topology, error);
 
-      /* The processing of the hello results above may have added/removed
-       * server descriptions. We need to reconcile that with our monitoring
-       * agents
+      /* The processing of the hello results above may have added, changed, or
+       * removed server descriptions. We need to reconcile that with our
+       * monitoring agents
        */
       mongoc_topology_reconcile (topology);
 
@@ -442,7 +449,7 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded)
    while (hl) {
       mongoc_topology_description_add_server (
          &topology->description, hl->host_and_port, &id);
-      mongoc_topology_scanner_add (topology->scanner, hl, id);
+      mongoc_topology_scanner_add (topology->scanner, hl, id, false);
 
       hl = hl->next;
    }

src/libmongoc/tests/test-happy-eyeballs.c

@@ -304,7 +304,7 @@ _testcase_run (he_testcase_t *testcase)
    testcase->state.start = bson_get_monotonic_time ();
 
    mongoc_topology_scanner_add (
-      ts, &testcase->state.host, 1 /* any server id is ok. */);
+      ts, &testcase->state.host, 1 /* any server id is ok. */, false);
    mongoc_topology_scanner_scan (ts, 1);
    /* how many commands should we have initially? */
    ASSERT_CMPINT ((int) (ts->async->ncmds), ==, expected->initial_acmds);

src/libmongoc/tests/test-mongoc-topology-scanner.c

@@ -92,7 +92,8 @@ _test_topology_scanner (bool with_ssl)
       mongoc_topology_scanner_add (
          topology_scanner,
          mongoc_uri_get_hosts (mock_server_get_uri (servers[i])),
-         (uint32_t) i);
+         (uint32_t) i,
+         false);
    }
 
    for (i = 0; i < 3; i++) {
@@ -498,7 +499,7 @@ test_topology_scanner_dns_testcase (dns_testcase_t *testcase)
    BSON_ASSERT (!host.next);
    bson_free (host_str);
 
-   mongoc_topology_scanner_add (ts, &host, 1);
+   mongoc_topology_scanner_add (ts, &host, 1, false);
    mongoc_topology_scanner_scan (ts, 1 /* any server id is ok. */);
    ASSERT_CMPINT ((int) (ts->async->ncmds), ==, testcase->expected_ncmds);
    mongoc_topology_scanner_work (ts);
@@ -603,7 +604,7 @@ test_topology_retired_fails_to_initiate (void)
    BSON_ASSERT (_mongoc_host_list_from_string (
       &host_list, mock_server_get_host_and_port (server)));
 
-   mongoc_topology_scanner_add (scanner, &host_list, 1);
+   mongoc_topology_scanner_add (scanner, &host_list, 1, false);
    mongoc_topology_scanner_start (scanner, false);
    BSON_ASSERT (scanner->async->ncmds > 0);
    /* retire the node */

src/libmongoc/tests/test-mongoc-topology.c

@@ -529,7 +529,7 @@ _test_topology_invalidate_server (bool pooled)
    fake_sd->type = MONGOC_SERVER_STANDALONE;
    mongoc_set_add (td->servers, fake_id, fake_sd);
    mongoc_topology_scanner_add (
-      client->topology->scanner, &fake_host_list, fake_id);
+      client->topology->scanner, &fake_host_list, fake_id, false);
    BSON_ASSERT (!mongoc_cluster_stream_for_server (
       &client->cluster, fake_id, true, NULL, NULL, &error));
    bson_mutex_lock (&client->topology->mutex);