CDRIVER-5643 Remove cluster time gossiping on SDAM commands (#2149)

Author: connorsmacd

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

@@ -252,18 +252,6 @@ _server_monitor_heartbeat_failed(mongoc_server_monitor_t *server_monitor,
    bson_mutex_unlock(&log_and_monitor->apm_mutex);
 }
 
-static void
-_server_monitor_append_cluster_time(mongoc_server_monitor_t *server_monitor, bson_t *cmd)
-{
-   mc_shared_tpld td = mc_tpld_take_ref(BSON_ASSERT_PTR_INLINE(server_monitor)->topology);
-
-   /* Cluster time is updated on every reply. */
-   if (!bson_empty(&td.ptr->cluster_time)) {
-      bson_append_document(cmd, "$clusterTime", 12, &td.ptr->cluster_time);
-   }
-   mc_tpld_drop_ref(&td);
-}
-
 static bool
 _server_monitor_send_and_recv_hello_opmsg(mongoc_server_monitor_t *server_monitor,
                                           const bson_t *cmd,
@@ -521,8 +509,6 @@ _server_monitor_polling_hello(mongoc_server_monitor_t *server_monitor,
    hello = _mongoc_topology_scanner_get_monitoring_cmd(server_monitor->topology->scanner, hello_ok);
    bson_copy_to(hello, &cmd);
 
-   _server_monitor_append_cluster_time(server_monitor, &cmd);
-
    ret = _server_monitor_send_and_recv(server_monitor, &cmd, hello_response, error);
 
    bson_destroy(&cmd);
@@ -782,7 +768,6 @@ _server_monitor_awaitable_hello(mongoc_server_monitor_t *server_monitor,
    hello = _mongoc_topology_scanner_get_monitoring_cmd(server_monitor->topology->scanner, description->hello_ok);
    bson_copy_to(hello, &cmd);
 
-   _server_monitor_append_cluster_time(server_monitor, &cmd);
    bson_append_document(&cmd, "topologyVersion", 15, &description->topology_version);
    bson_append_int64(&cmd, "maxAwaitTimeMS", 14, server_monitor->heartbeat_frequency_ms);
    bson_append_utf8(&cmd, "$db", 3, "admin", 5);
@@ -823,10 +808,6 @@ _update_topology_description(mongoc_server_monitor_t *server_monitor, mongoc_ser
       hello_response = &description->last_hello_response;
    }
 
-   if (hello_response) {
-      _mongoc_topology_update_cluster_time(topology, hello_response);
-   }
-
    if (mcommon_atomic_int_fetch(&topology->scanner_state, mcommon_memory_order_relaxed) ==
        MONGOC_TOPOLOGY_SCANNER_SHUTTING_DOWN) {
       return;
@@ -841,6 +822,7 @@ _update_topology_description(mongoc_server_monitor_t *server_monitor, mongoc_ser
                                             server_monitor->server_id,
                                             hello_response,
                                             description->round_trip_time_msec,
+                                            MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
                                             &description->error);
    /* Reconcile server monitors. */
    _mongoc_topology_background_monitoring_reconcile(topology, tdmod.new_td);
@@ -965,7 +947,6 @@ _server_monitor_setup_connection(mongoc_server_monitor_t *server_monitor,
    /* Perform handshake. */
    bson_destroy(&cmd);
    _mongoc_topology_dup_handshake_cmd(server_monitor->topology, &cmd);
-   _server_monitor_append_cluster_time(server_monitor, &cmd);
    bson_destroy(hello_response);
 
    ret = _server_monitor_send_and_recv(server_monitor, &cmd, hello_response, error);

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

@@ -106,13 +106,20 @@ _mongoc_topology_description_copy_to(const mongoc_topology_description_t *src, m
 void
 mongoc_topology_description_cleanup(mongoc_topology_description_t *description);
 
+typedef enum {
+   MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
+   MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_UPDATE,
+} mongoc_topology_description_hello_cluster_time_strategy_t;
+
 void
-mongoc_topology_description_handle_hello(mongoc_topology_description_t *topology,
-                                         const mongoc_log_and_monitor_instance_t *log_and_monitor,
-                                         uint32_t server_id,
-                                         const bson_t *hello_response,
-                                         int64_t rtt_msec,
-                                         const bson_error_t *error /* IN */);
+mongoc_topology_description_handle_hello(
+   mongoc_topology_description_t *topology,
+   const mongoc_log_and_monitor_instance_t *log_and_monitor,
+   uint32_t server_id,
+   const bson_t *hello_response,
+   int64_t rtt_msec,
+   mongoc_topology_description_hello_cluster_time_strategy_t cluster_time_strategy,
+   const bson_error_t *error /* IN */);
 
 mongoc_server_description_t const *
 mongoc_topology_description_select(const mongoc_topology_description_t *description,

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

@@ -1353,7 +1353,8 @@ mongoc_topology_description_invalidate_server(mongoc_topology_description_t *td,
    }
 
    /* send NULL hello reply */
-   mongoc_topology_description_handle_hello(td, log_and_monitor, id, NULL, MONGOC_RTT_UNSET, error);
+   mongoc_topology_description_handle_hello(
+      td, log_and_monitor, id, NULL, MONGOC_RTT_UNSET, MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE, error);
 }
 
 /*
@@ -2129,12 +2130,14 @@ _mongoc_topology_description_check_compatible(mongoc_topology_description_t *td)
  */
 
 void
-mongoc_topology_description_handle_hello(mongoc_topology_description_t *topology,
-                                         const mongoc_log_and_monitor_instance_t *log_and_monitor,
-                                         uint32_t server_id,
-                                         const bson_t *hello_response,
-                                         int64_t rtt_msec,
-                                         const bson_error_t *error /* IN */)
+mongoc_topology_description_handle_hello(
+   mongoc_topology_description_t *topology,
+   const mongoc_log_and_monitor_instance_t *log_and_monitor,
+   uint32_t server_id,
+   const bson_t *hello_response,
+   int64_t rtt_msec,
+   mongoc_topology_description_hello_cluster_time_strategy_t cluster_time_strategy,
+   const bson_error_t *error /* IN */)
 {
    mongoc_topology_description_t *prev_td = NULL;
    mongoc_server_description_t *prev_sd = NULL;
@@ -2218,7 +2221,9 @@ mongoc_topology_description_handle_hello(mongoc_topology_description_t *topology
       }
    }
 
-   mongoc_topology_description_update_cluster_time(topology, hello_response);
+   if (cluster_time_strategy == MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_UPDATE) {
+      mongoc_topology_description_update_cluster_time(topology, hello_response);
+   }
 
    if (prev_sd) {
       sd_changed = !_mongoc_server_description_equal(prev_sd, sd);

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

@@ -117,7 +117,6 @@ typedef struct mongoc_topology_scanner {
    bson_mutex_t handshake_cmd_mtx;
    bson_t *handshake_cmd;
    handshake_state_t handshake_state;
-   bson_t cluster_time;
    bson_oid_t topology_id;
    const char *appname;
 
@@ -242,8 +241,6 @@ void
 mongoc_topology_scanner_set_stream_initiator(mongoc_topology_scanner_t *ts, mongoc_stream_initiator_t si, void *ctx);
 bool
 _mongoc_topology_scanner_set_appname(mongoc_topology_scanner_t *ts, const char *name);
-void
-_mongoc_topology_scanner_set_cluster_time(mongoc_topology_scanner_t *ts, const bson_t *cluster_time);
 
 void
 _mongoc_topology_scanner_set_dns_cache_timeout(mongoc_topology_scanner_t *ts, int64_t timeout_ms);

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

@@ -169,7 +169,6 @@ _init_hello(mongoc_topology_scanner_t *ts)
 {
    bson_init(&ts->hello_cmd);
    bson_init(&ts->legacy_hello_cmd);
-   bson_init(&ts->cluster_time);
    ts->handshake_cmd = NULL;
 
    _add_hello(ts);
@@ -441,10 +440,6 @@ _begin_hello_cmd(mongoc_topology_scanner_node_t *node,
       bson_free(oidc_access_token);
    }
 
-   if (!bson_empty(&ts->cluster_time)) {
-      bson_append_document(&cmd, "$clusterTime", 12, &ts->cluster_time);
-   }
-
    /* if the node should connect with a TCP socket, stream will be null, and
     * dns_result will be set. The async loop is responsible for calling the
     * _tcp_initiator to construct TCP sockets. */
@@ -533,7 +528,6 @@ mongoc_topology_scanner_destroy(mongoc_topology_scanner_t *ts)
    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);
    bson_mutex_destroy(&ts->handshake_cmd_mtx);
 
@@ -1343,17 +1337,6 @@ _mongoc_topology_scanner_set_appname(mongoc_topology_scanner_t *ts, const char *
    return false;
 }
 
-/*
- * Set the scanner's clusterTime unconditionally: don't compare with prior
- * @cluster_time is like {clusterTime: <timestamp>}
- */
-void
-_mongoc_topology_scanner_set_cluster_time(mongoc_topology_scanner_t *ts, const bson_t *cluster_time)
-{
-   bson_destroy(&ts->cluster_time);
-   bson_copy_to(cluster_time, &ts->cluster_time);
-}
-
 /* SDAM Monitoring Spec: send HeartbeatStartedEvent */
 static void
 _mongoc_topology_scanner_monitor_heartbeat_started(const mongoc_topology_scanner_t *ts, const mongoc_host_list_t *host)

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

@@ -95,11 +95,13 @@ static bool
 _mongoc_topology_update_no_lock(uint32_t id,
                                 const bson_t *hello_response,
                                 int64_t rtt_msec,
+                                mongoc_topology_description_hello_cluster_time_strategy_t cluster_time_strategy,
                                 mongoc_topology_description_t *td,
                                 const mongoc_log_and_monitor_instance_t *log_and_monitor,
                                 const bson_error_t *error /* IN */)
 {
-   mongoc_topology_description_handle_hello(td, log_and_monitor, id, hello_response, rtt_msec, error);
+   mongoc_topology_description_handle_hello(
+      td, log_and_monitor, id, hello_response, rtt_msec, cluster_time_strategy, error);
 
    /* return false if server removed from topology */
    return mongoc_topology_description_server_by_id(td, id, NULL) != NULL;
@@ -130,8 +132,13 @@ _mongoc_topology_scanner_setup_err_cb(uint32_t id, void *data, const bson_error_
       // Use `mc_tpld_unsafe_get_mutable` to get a mutable topology description
       // without locking. This function only applies to single-threaded clients.
       mongoc_topology_description_t *td = mc_tpld_unsafe_get_mutable(topology);
-      mongoc_topology_description_handle_hello(
-         td, &topology->log_and_monitor, id, NULL /* hello reply */, -1 /* rtt_msec */, error);
+      mongoc_topology_description_handle_hello(td,
+                                               &topology->log_and_monitor,
+                                               id,
+                                               NULL /* hello reply */,
+                                               -1 /* rtt_msec */,
+                                               MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
+                                               error);
    }
 }
 
@@ -181,13 +188,25 @@ _mongoc_topology_scanner_cb(
     * client MUST change its type to Unknown only after it has retried the
     * server once." */
    if (!hello_response && sd && sd->type != MONGOC_SERVER_UNKNOWN) {
-      _mongoc_topology_update_no_lock(id, hello_response, rtt_msec, td, &topology->log_and_monitor, error);
+      _mongoc_topology_update_no_lock(id,
+                                      hello_response,
+                                      rtt_msec,
+                                      MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
+                                      td,
+                                      &topology->log_and_monitor,
+                                      error);
 
       /* add another hello call to the current scan - the scan continues
        * until all commands are done */
       mongoc_topology_scanner_scan(topology->scanner, sd->id);
    } else {
-      _mongoc_topology_update_no_lock(id, hello_response, rtt_msec, td, &topology->log_and_monitor, error);
+      _mongoc_topology_update_no_lock(id,
+                                      hello_response,
+                                      rtt_msec,
+                                      MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
+                                      td,
+                                      &topology->log_and_monitor,
+                                      error);
 
       /* The processing of the hello results above may have added, changed, or
        * removed server descriptions. We need to reconcile that with our
@@ -1444,8 +1463,13 @@ _mongoc_topology_update_from_handshake(mongoc_topology_t *topology, const mongoc
    tdmod = mc_tpld_modify_begin(topology);
 
    /* return false if server was removed from topology */
-   has_server = _mongoc_topology_update_no_lock(
-      sd->id, &sd->last_hello_response, sd->round_trip_time_msec, tdmod.new_td, &topology->log_and_monitor, NULL);
+   has_server = _mongoc_topology_update_no_lock(sd->id,
+                                                &sd->last_hello_response,
+                                                sd->round_trip_time_msec,
+                                                MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_UPDATE,
+                                                tdmod.new_td,
+                                                &topology->log_and_monitor,
+                                                NULL);
 
    /* if pooled, wake threads waiting in mongoc_topology_server_by_id */
    mongoc_cond_broadcast(&topology->cond_client);
@@ -1557,7 +1581,6 @@ _mongoc_topology_update_cluster_time(mongoc_topology_t *topology, const bson_t *
           _mongoc_cluster_time_greater(&cluster_time, &tdmod.new_td->cluster_time)) {
          bson_destroy(&tdmod.new_td->cluster_time);
          bson_copy_to(&cluster_time, &tdmod.new_td->cluster_time);
-         _mongoc_topology_scanner_set_cluster_time(topology->scanner, &tdmod.new_td->cluster_time);
          mc_tpld_modify_commit(tdmod);
       } else {
          mc_tpld_modify_drop(tdmod);

src/libmongoc/tests/json-test.c

@@ -224,7 +224,13 @@ process_sdam_test_hello_responses(bson_t *phase, mongoc_topology_t *topology)
 
          /* send hello through the topology description's handler */
          capture_logs(true);
-         mongoc_topology_description_handle_hello(tdmod.new_td, &topology->log_and_monitor, sd->id, &response, 1, NULL);
+         mongoc_topology_description_handle_hello(tdmod.new_td,
+                                                  &topology->log_and_monitor,
+                                                  sd->id,
+                                                  &response,
+                                                  1,
+                                                  MONGOC_TOPOLOGY_DESCRIPTION_HELLO_CLUSTER_TIME_IGNORE,
+                                                  NULL);
          if (mc_tpld_servers_const(tdmod.new_td)->items_len == 0) {
             ASSERT_CAPTURED_LOG("topology", MONGOC_LOG_LEVEL_WARNING, "Last server removed from topology");
          }

src/libmongoc/tests/test-mongoc-client-session.c

@@ -1043,7 +1043,12 @@ started(const mongoc_apm_command_started_t *event)
    }
 
    has_cluster_time = bson_iter_init_find(&iter, cmd, "$clusterTime");
-   if (test->acknowledged && !has_cluster_time) {
+
+   // Since $clusterTime is no longer used with SDAM, there may not be a cluster time until the first successful
+   // command completes.
+   const bool should_have_cluster_time = test->acknowledged && test->n_succeeded >= 1;
+
+   if (should_have_cluster_time && !has_cluster_time) {
       test_error("no $clusterTime sent with command %s", cmd_name);
    }