CDRIVER-751 don't recheck any host in same scan

Fix test_topology_scanner_oscillate case.

Author: ajdavis

CMakeLists.txt

@@ -101,6 +101,7 @@ set (SOURCES
    ${SOURCE_DIR}/src/mongoc/mongoc-gridfs-file-list.c
    ${SOURCE_DIR}/src/mongoc/mongoc-gridfs-file-page.c
    ${SOURCE_DIR}/src/mongoc/mongoc-gridfs-file-list.c
+   ${SOURCE_DIR}/src/mongoc/mongoc-host-list.c
    ${SOURCE_DIR}/src/mongoc/mongoc-index.c
    ${SOURCE_DIR}/src/mongoc/mongoc-init.c
    ${SOURCE_DIR}/src/mongoc/mongoc-list.c

src/mongoc/Makefile.am

@@ -120,6 +120,7 @@ MONGOC_SOURCES_SHARED += \
 	src/mongoc/mongoc-cursor-cursorid.c \
 	src/mongoc/mongoc-cursor-transform.c \
 	src/mongoc/mongoc-database.c \
+	src/mongoc/mongoc-host-list.c \
 	src/mongoc/mongoc-init.c \
 	src/mongoc/mongoc-gridfs.c \
 	src/mongoc/mongoc-gridfs-file.c \

src/mongoc/mongoc-host-list.c

@@ -0,0 +1,172 @@
+/*
+ * Copyright 2015 MongoDB Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "mongoc-host-list.h"
+
+
+#ifdef _WIN32
+# define strcasecmp _stricmp
+#endif
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_host_list_new --
+ *
+ *       Empty new host list.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+mongoc_host_list_t *
+mongoc_host_list_new (void)
+{
+   return (mongoc_host_list_t *) bson_malloc0 (sizeof (mongoc_host_list_t));
+}
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_host_list_equal --
+ *
+ *       Check two hosts have the same domain (case-insensitive), port,
+ *       and address family.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+bool
+mongoc_host_list_equal (const mongoc_host_list_t *host_a,
+                        const mongoc_host_list_t *host_b)
+{
+   return (!strcasecmp (host_a->host_and_port, host_b->host_and_port)
+           && host_a->family == host_b->family);
+}
+
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_host_list_find --
+ *
+ *       Search for an equal mongoc_host_list_t in a list of them.
+ *
+ * Returns:
+ *       Pointer to an entry in "list", or NULL.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+const mongoc_host_list_t *
+mongoc_host_list_find (const mongoc_host_list_t *list,
+                       const mongoc_host_list_t *needle)
+{
+   while (list) {
+      if (mongoc_host_list_equal (list, needle)) {
+         return list;
+      }
+
+      list = list->next;
+   }
+
+   return NULL;
+}
+
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_host_list_count --
+ *
+ *       Return number of items in the host list.
+ *
+ * Returns:
+ *       0 or greater.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+size_t
+mongoc_host_list_count (const mongoc_host_list_t *list)
+{
+   size_t count = 0;
+
+   while (list) {
+      count++;
+      list = list->next;
+   }
+
+   return count;
+}
+
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * host_list_copy --
+ *
+ *       Make a copy of "host" with next set to "next".
+ *
+ * Returns:
+ *       New copy.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+mongoc_host_list_t *
+mongoc_host_list_copy (const mongoc_host_list_t *host,
+                       mongoc_host_list_t *next)
+{
+   mongoc_host_list_t *copy;
+
+   copy = mongoc_host_list_new ();
+   memcpy ((void *) copy, (void *) host, sizeof (mongoc_host_list_t));
+   copy->next = next;
+
+   return copy;
+}
+
+
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_host_list_destroy_all --
+ *
+ *       Destroy whole linked list of hosts.
+ *
+ *--------------------------------------------------------------------------
+ */
+void
+mongoc_host_list_destroy_all (mongoc_host_list_t *host)
+{
+   mongoc_host_list_t *tmp;
+
+   while (host) {
+      tmp = host->next;
+      bson_free (host);
+      host = tmp;
+   }
+}

src/mongoc/mongoc-host-list.h

@@ -51,6 +51,20 @@ struct _mongoc_host_list_t
    void               *padding [4];
 };
 
+mongoc_host_list_t *mongoc_host_list_new (void);
+
+bool mongoc_host_list_equal (const mongoc_host_list_t *host_a,
+                             const mongoc_host_list_t *host_b);
+
+const mongoc_host_list_t *mongoc_host_list_find (const mongoc_host_list_t *list,
+                                                 const mongoc_host_list_t *needle);
+
+size_t mongoc_host_list_count (const mongoc_host_list_t *list);
+
+mongoc_host_list_t *mongoc_host_list_copy (const mongoc_host_list_t *host,
+                                           mongoc_host_list_t *next);
+
+void mongoc_host_list_destroy_all (mongoc_host_list_t *host);
 
 BSON_END_DECLS
 

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

@@ -69,6 +69,7 @@ typedef struct mongoc_topology_scanner
    mongoc_async_cmd_setup_t        setup;
    mongoc_stream_initiator_t       initiator;
    void                           *initiator_context;
+   mongoc_host_list_t             *seen;
 
 #ifdef MONGOC_ENABLE_SSL
    mongoc_ssl_opt_t *ssl_opts;

src/mongoc/mongoc-topology-scanner.c

@@ -92,6 +92,8 @@ mongoc_topology_scanner_destroy (mongoc_topology_scanner_t *ts)
    mongoc_async_destroy (ts->async);
    bson_destroy (&ts->ismaster_cmd);
 
+   mongoc_host_list_destroy_all (ts->seen);
+
    bson_free (ts);
 }
 
@@ -100,7 +102,18 @@ _add_node (mongoc_topology_scanner_t *ts,
            const mongoc_host_list_t  *host,
            uint32_t                   id)
 {
-   mongoc_topology_scanner_node_t *node = (mongoc_topology_scanner_node_t *)bson_malloc0 (sizeof (*node));
+   mongoc_topology_scanner_node_t *node;
+
+   if (mongoc_host_list_find (ts->seen, host)) {
+      /* already seen this host -- avoid pathological case like two primaries
+       * pointing at each other, each invalidating itself and trying to add the
+       * other on each check within the same blocking scan, see
+       * test_topology_scanner_oscillate
+       */
+      return NULL;
+   }
+
+   node = (mongoc_topology_scanner_node_t *) bson_malloc0 (sizeof (*node));
 
    memcpy (&node->host, host, sizeof (*host));
 
@@ -110,6 +123,8 @@ _add_node (mongoc_topology_scanner_t *ts,
 
    DL_APPEND(ts->nodes, node);
 
+   ts->seen = mongoc_host_list_copy (host, ts->seen);
+
    return node;
 }
 
@@ -132,7 +147,7 @@ mongoc_topology_scanner_add_and_scan (mongoc_topology_scanner_t *ts,
 
    node = _add_node (ts, host, id);
 
-   if (mongoc_topology_scanner_node_setup (node)) {
+   if (node && mongoc_topology_scanner_node_setup (node)) {
       node->cmd = mongoc_async_cmd (
          ts->async, node->stream, ts->setup,
          node->host.host, "admin",
@@ -147,6 +162,7 @@ mongoc_topology_scanner_add_and_scan (mongoc_topology_scanner_t *ts,
 void
 mongoc_topology_scanner_node_destroy (mongoc_topology_scanner_node_t *node, bool failed)
 {
+   /* delete from nodes but keep its host in "seen" so we don't rescan */
    DL_DELETE (node->ts->nodes, node);
 
    if (node->dns_results) {
@@ -491,6 +507,8 @@ mongoc_topology_scanner_work (mongoc_topology_scanner_t *ts,
    r = mongoc_async_run (ts->async, timeout_msec);
 
    if (! r) {
+      mongoc_host_list_destroy_all (ts->seen);
+      ts->seen = NULL;
       ts->in_progress = false;
    }
 

tests/TestSuite.h

@@ -101,7 +101,7 @@ extern "C" {
 #define AWAIT(_condition) \
    do { \
       int64_t _start = bson_get_monotonic_time (); \
-      while (! _condition) { \
+      while (! (_condition)) { \
          if (bson_get_monotonic_time() - _start > 1000 * 1000) { \
             fprintf (stderr, \
                      "%s:%d %s(): \"%s\" still false after 1 second\n", \

tests/test-mongoc-topology-scanner.c

@@ -142,7 +142,7 @@ test_topology_scanner_discovery ()
    mongoc_client_t *client;
    mongoc_topology_scanner_t *scanner;
    char *uri_str;
-   mongoc_read_prefs_t *primary_pref;
+   mongoc_read_prefs_t *secondary_pref;
    bson_error_t error;
    future_t *future;
    request_t *request;
@@ -174,32 +174,41 @@ test_topology_scanner_discovery ()
                                  mock_server_get_host_and_port (primary));
    client = mongoc_client_new (uri_str);
    scanner = client->topology->scanner;
-   primary_pref = mongoc_read_prefs_new (MONGOC_READ_PRIMARY);
+   secondary_pref = mongoc_read_prefs_new (MONGOC_READ_SECONDARY_PREFERRED);
 
    future = future_topology_select (client->topology, MONGOC_SS_READ,
-                                    primary_pref, 15, &error);
+                                    secondary_pref, 15, &error);
 
    /* a single scan discovers *and* checks the secondary */
+   AWAIT (scanner->async->ncmds == 1);
+   ASSERT_CMPINT (1, ==, (int) mongoc_host_list_count (scanner->seen));
    request = mock_server_receives_ismaster (primary);
    mock_server_replies_simple (request, primary_response);
    request_destroy (request);
 
    /* let client process that response */
    _mongoc_usleep (250 * 1000);
+   ASSERT_CMPINT (2, ==, (int) mongoc_host_list_count (scanner->seen));
 
    /* a check of the secondary is scheduled in this scan */
-   AWAIT (scanner->async->ncmds);
+   AWAIT (scanner->async->ncmds == 1);
 
    request = mock_server_receives_ismaster (secondary);
    mock_server_replies_simple (request, secondary_response);
    /* scan completes */
-   AWAIT (!scanner->async->ncmds);
+   AWAIT (scanner->async->ncmds == 0);
    ASSERT_OR_PRINT ((sd = future_get_mongoc_server_description_ptr (future)),
                     error);
 
+   ASSERT_CMPSTR (sd->host.host_and_port,
+                  mock_server_get_host_and_port (secondary));
+
+   /* "seen" is reset */
+   ASSERT_CMPINT (0, ==, (int) mongoc_host_list_count (scanner->seen));
+
    mongoc_server_description_destroy (sd);
    request_destroy (request);
-   mongoc_read_prefs_destroy (primary_pref);
+   mongoc_read_prefs_destroy (secondary_pref);
    bson_free (secondary_response);
    bson_free (primary_response);
    bson_free (uri_str);
@@ -209,9 +218,7 @@ test_topology_scanner_discovery ()
 }
 
 
-/*
- * Check that scanner doesn't spinlock if two primaries point at each other.
- */
+/* scanner shouldn't spin if two primaries point at each other */
 void
 test_topology_scanner_oscillate ()
 {
@@ -266,13 +273,21 @@ test_topology_scanner_oscillate ()
 
    /* let client process that response */
    _mongoc_usleep (250 * 1000);
-   AWAIT (scanner->async->ncmds);
+   AWAIT (scanner->async->ncmds == 1);
 
    request = mock_server_receives_ismaster (server1);
    mock_server_replies_simple (request, server1_response);
-   AWAIT (!scanner->async->ncmds);
+
+   /* we don't schedule another check of server0 */
+   _mongoc_usleep (250 * 1000);
+   AWAIT (scanner->async->ncmds == 0);
 
    assert (!future_get_mongoc_server_description_ptr (future));
+   assert (scanner->async->ncmds == 0);
+
+   /* "seen" is reset */
+   ASSERT_CMPINT (0, ==, (int) mongoc_host_list_count (scanner->seen));
+
    request_destroy (request);
    mongoc_read_prefs_destroy (primary_pref);
    bson_free (server1_response);
@@ -293,8 +308,6 @@ test_topology_scanner_install (TestSuite *suite)
 #endif
    TestSuite_Add (suite, "/TOPOLOGY/scanner_discovery",
                   test_topology_scanner_discovery);
-/*
    TestSuite_Add (suite, "/TOPOLOGY/scanner_oscillate",
                   test_topology_scanner_oscillate);
-*/
 }