Use hello command after seeing helloOk in legacy hello response

Author: alcaeus

src/libmongoc/src/mongoc/mongoc-server-monitor.c

@@ -329,8 +329,8 @@ _server_monitor_polling_hello (mongoc_server_monitor_t *server_monitor,
    const bson_t *hello;
    bool ret;
 
-   hello = _mongoc_topology_scanner_get_hello_cmd (
-      server_monitor->topology->scanner);
+   hello = _mongoc_topology_scanner_get_monitoring_cmd (
+      server_monitor->topology->scanner, server_monitor->description->hello_ok);
    bson_copy_to (hello, &cmd);
 
    _server_monitor_append_cluster_time (server_monitor, &cmd);
@@ -609,8 +609,8 @@ _server_monitor_awaitable_hello (mongoc_server_monitor_t *server_monitor,
    const bson_t *hello;
    bool ret = false;
 
-   hello = _mongoc_topology_scanner_get_hello_cmd (
-      server_monitor->topology->scanner);
+   hello = _mongoc_topology_scanner_get_monitoring_cmd (
+      server_monitor->topology->scanner, server_monitor->description->hello_ok);
    bson_copy_to (hello, &cmd);
 
    _server_monitor_append_cluster_time (server_monitor, &cmd);
@@ -1052,6 +1052,16 @@ static BSON_THREAD_FUN (_server_monitor_thread, server_monitor_void)
          continue;
       }
 
+      /* Update the server description of the server monitor to ensure we're
+       * running the correct heartbeat command in the future */
+      if (description->has_hello_response) {
+         mongoc_server_description_handle_hello (
+            server_monitor->description,
+            &description->last_hello_response,
+            description->round_trip_time_msec,
+            &description->error);
+      }
+
       _server_monitor_update_topology_description (server_monitor, description);
 
       /* Immediately proceed to the next check if the previous response was

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;
 
@@ -84,6 +85,7 @@ typedef struct mongoc_topology_scanner {
    int64_t connect_timeout_msec;
    mongoc_topology_scanner_node_t *nodes;
    bson_t hello_cmd;
+   bson_t legacy_hello_cmd;
    bson_t handshake_cmd;
    bson_t cluster_time;
    bool handshake_ok_to_send;
@@ -130,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);
@@ -194,7 +197,8 @@ _mongoc_topology_scanner_get_speculative_auth_mechanism (
    const mongoc_uri_t *uri);
 
 const bson_t *
-_mongoc_topology_scanner_get_hello_cmd (mongoc_topology_scanner_t *ts);
+_mongoc_topology_scanner_get_monitoring_cmd (mongoc_topology_scanner_t *ts,
+                                             bool hello_ok);
 
 const bson_t *
 _mongoc_topology_scanner_get_handshake_cmd (mongoc_topology_scanner_t *ts);

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

@@ -106,22 +106,27 @@ _jumpstart_other_acmds (mongoc_topology_scanner_node_t *node,
 static void
 _add_hello (mongoc_topology_scanner_t *ts)
 {
-   bson_t *cmd = &ts->hello_cmd;
    mongoc_server_api_t *api = ts->api;
 
+   BSON_APPEND_INT32 (&ts->hello_cmd, "hello", 1);
+   BSON_APPEND_BOOL (&ts->hello_cmd, "helloOk", true);
+
    if (api) {
-      BSON_APPEND_INT32 (cmd, "hello", 1);
-      _mongoc_cmd_append_server_api (cmd, api);
+      BSON_APPEND_INT32 (&ts->legacy_hello_cmd, "hello", 1);
+
+      _mongoc_cmd_append_server_api (&ts->hello_cmd, api);
+      _mongoc_cmd_append_server_api (&ts->legacy_hello_cmd, api);
    } else {
-      BSON_APPEND_INT32 (cmd, HANDSHAKE_CMD_LEGACY_HELLO, 1);
-      BSON_APPEND_BOOL (cmd, "helloOk", true);
+      BSON_APPEND_INT32 (&ts->legacy_hello_cmd, HANDSHAKE_CMD_LEGACY_HELLO, 1);
+      BSON_APPEND_BOOL (&ts->legacy_hello_cmd, "helloOk", true);
    }
 }
 
 static void
 _init_hello (mongoc_topology_scanner_t *ts)
 {
    bson_init (&ts->hello_cmd);
+   bson_init (&ts->legacy_hello_cmd);
    bson_init (&ts->handshake_cmd);
    bson_init (&ts->cluster_time);
 
@@ -132,6 +137,7 @@ static void
 _reset_hello (mongoc_topology_scanner_t *ts)
 {
    bson_reinit (&ts->hello_cmd);
+   bson_reinit (&ts->legacy_hello_cmd);
    bson_reinit (&ts->handshake_cmd);
 
    _add_hello (ts);
@@ -255,7 +261,7 @@ _build_handshake_cmd (mongoc_topology_scanner_t *ts)
    char buf[16];
 
    bson_destroy (doc);
-   bson_copy_to (&ts->hello_cmd, doc);
+   bson_copy_to (&ts->legacy_hello_cmd, doc);
 
    BSON_APPEND_DOCUMENT_BEGIN (doc, HANDSHAKE_FIELD, &subdoc);
    res = _mongoc_handshake_build_doc_with_application (&subdoc, ts->appname);
@@ -280,9 +286,10 @@ _build_handshake_cmd (mongoc_topology_scanner_t *ts)
 }
 
 const bson_t *
-_mongoc_topology_scanner_get_hello_cmd (mongoc_topology_scanner_t *ts)
+_mongoc_topology_scanner_get_monitoring_cmd (mongoc_topology_scanner_t *ts,
+                                             bool hello_ok)
 {
-   return &ts->hello_cmd;
+   return hello_ok ? &ts->hello_cmd : &ts->legacy_hello_cmd;
 }
 
 /* Caller must lock topology->mutex to protect handshake_cmd. This
@@ -303,7 +310,7 @@ _mongoc_topology_scanner_get_handshake_cmd (mongoc_topology_scanner_t *ts)
 
    /* If the doc turned out to be too big */
    if (!ts->handshake_ok_to_send) {
-      return &ts->hello_cmd;
+      return &ts->legacy_hello_cmd;
    }
 
    return &ts->handshake_cmd;
@@ -321,7 +328,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 (_mongoc_topology_scanner_get_monitoring_cmd (ts, node->hello_ok),
+                    &cmd);
    } else {
       bson_copy_to (_mongoc_topology_scanner_get_handshake_cmd (ts), &cmd);
    }
@@ -429,6 +437,7 @@ mongoc_topology_scanner_destroy (mongoc_topology_scanner_t *ts)
 
    mongoc_async_destroy (ts->async);
    bson_destroy (&ts->hello_cmd);
+   bson_destroy (&ts->legacy_hello_cmd);
    bson_destroy (&ts->handshake_cmd);
    bson_destroy (&ts->cluster_time);
    mongoc_server_api_destroy (ts->api);
@@ -450,7 +459,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;
 
@@ -462,6 +472,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 */