SERVER-70191 Share HostAndPort alphabetization code for test between async_rpc and NetworkInterface

12 files changed, 74 insertions, 93 deletions

diff --git a/jstests/sharding/hedged_reads.js b/jstests/sharding/hedged_reads.js
index 69c1e902563..3e1f24901f3 100644
--- a/jstests/sharding/hedged_reads.js
+++ b/jstests/sharding/hedged_reads.js
@@ -1,6 +1,6 @@
 /**
  * Tests hedging metrics in the serverStatus output.
- * @tags: [requires_fcv_62]
+ * @tags: [requires_fcv_70]
  */
 (function() {
 "use strict";
@@ -103,7 +103,7 @@ const st = new ShardingTest({
             // Force the mongos's replica set monitors to always include all the eligible nodes.
             "failpoint.sdamServerSelectorIgnoreLatencyWindow": tojson({mode: "alwaysOn"}),
             // Force the mongos to send requests to hosts in alphabetical order of host names.
-            "failpoint.networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder":
+            "failpoint.hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder":
                 tojson({mode: "alwaysOn"}),
             maxTimeMSForHedgedReads: 10
         }
diff --git a/jstests/sharding/hedging_metrics_server_status.js b/jstests/sharding/hedging_metrics_server_status.js
index ae93eb9aa75..55d0eaee19d 100644
--- a/jstests/sharding/hedging_metrics_server_status.js
+++ b/jstests/sharding/hedging_metrics_server_status.js
@@ -1,5 +1,6 @@
 /**
  * Tests hedging metrics in the serverStatus output.
+ * @tags: [requires_fcv_70]
  */
 (function() {
 "use strict";
@@ -70,7 +71,7 @@ const st = new ShardingTest({
             // Force the mongos's replica set monitors to always include all the eligible nodes.
             "failpoint.sdamServerSelectorIgnoreLatencyWindow": tojson({mode: "alwaysOn"}),
             // Force the mongos to send requests to hosts in alphabetical order of host names.
-            "failpoint.networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder":
+            "failpoint.hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder":
                 tojson({mode: "alwaysOn"}),
             maxTimeMSForHedgedReads: 500
         }
@@ -98,7 +99,7 @@ let serverSelectorFailPoint = configureFailPoint(st.s, "sdamServerSelectorIgnore
 
 // Force the mongos to send requests to hosts in alphabetical order of host names.
 let sendRequestsFailPoint =
-    configureFailPoint(st.s, "networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder");
+    configureFailPoint(st.s, "hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder");
 let sortedNodes = [...st.rs0.nodes].sort((node1, node2) => node1.host.localeCompare(node2.host));
 
 let expectedHedgingMetrics = {
diff --git a/src/mongo/executor/SConscript b/src/mongo/executor/SConscript
index d1a0f8aca9e..ba89e50deaf 100644
--- a/src/mongo/executor/SConscript
+++ b/src/mongo/executor/SConscript
@@ -175,6 +175,7 @@ env.Library(
         '$BUILD_DIR/mongo/db/server_feature_flags',
         '$BUILD_DIR/mongo/transport/transport_layer_manager',
         'connection_pool_executor',
+        'hedge_options_util',
         'network_interface',
     ],
 )
@@ -341,7 +342,6 @@ env.CppUnitTest(
         'mock_network_fixture_test.cpp',
         'network_interface_mock_test.cpp',
         'network_interface_mock_test_fixture.cpp',
-        'network_interface_tl_test.cpp',
         'scoped_task_executor_test.cpp',
         'split_timer_test.cpp',
         'task_executor_cursor_test.cpp',
diff --git a/src/mongo/executor/hedge_options_util.cpp b/src/mongo/executor/hedge_options_util.cpp
index 9d3184fda83..598ae2d1a20 100644
--- a/src/mongo/executor/hedge_options_util.cpp
+++ b/src/mongo/executor/hedge_options_util.cpp
@@ -35,6 +35,7 @@
 #include "mongo/util/optional_util.h"
 
 namespace mongo {
+MONGO_FAIL_POINT_DEFINE(hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder);
 namespace {
 // Only do hedging for commands that cannot trigger writes.
 constexpr std::array hedgeCommands{
@@ -66,8 +67,28 @@ bool commandShouldHedge(StringData command, const ReadPreferenceSetting& readPre
     }
     return commandCanHedge(command);
 }
+
+template <typename IA, typename IB, typename F>
+int compareTransformed(IA a1, IA a2, IB b1, IB b2, F&& f) {
+    for (;; ++a1, ++b1)
+        if (a1 == a2)
+            return b1 == b2 ? 0 : -1;
+        else if (b1 == b2)
+            return 1;
+        else if (int r = f(*a1) - f(*b1))
+            return r;
+}
 }  // namespace
 
+bool compareByLowerHostThenPort(const HostAndPort& a, const HostAndPort& b) {
+    const auto& ah = a.host();
+    const auto& bh = b.host();
+    if (int r = compareTransformed(
+            ah.begin(), ah.end(), bh.begin(), bh.end(), [](auto&& c) { return ctype::toLower(c); }))
+        return r < 0;
+    return a.port() < b.port();
+}
+
 HedgeOptions getHedgeOptions(StringData command, const ReadPreferenceSetting& readPref) {
     bool shouldHedge = commandShouldHedge(command, readPref);
     size_t hedgeCount = shouldHedge ? 1 : 0;
diff --git a/src/mongo/executor/hedge_options_util.h b/src/mongo/executor/hedge_options_util.h
index ce5fc63e9e2..7a6517e9f91 100644
--- a/src/mongo/executor/hedge_options_util.h
+++ b/src/mongo/executor/hedge_options_util.h
@@ -33,6 +33,9 @@
 #include "mongo/client/read_preference.h"
 
 namespace mongo {
+/** Failpoint used to test hedged reads. */
+extern FailPoint hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder;
+
 /**
  * HedgeOptions contains the information necessary for a particular command invocation to execute as
  * a hedged read. Specifically, it contains:
@@ -75,4 +78,12 @@ inline bool isIgnorableAsHedgeResult(const Status& status) {
     return status == ErrorCodes::MaxTimeMSExpired || status == ErrorCodes::StaleDbVersion ||
         ErrorCodes::isNetworkTimeoutError(status) || ErrorCodes::isStaleShardVersionError(status);
 }
+
+/**
+ * Orders HostAndPorts alphabetically; can be used by std::sort to order
+ * a range of HostAndPorts. We sort HostAndPorts alphabetically under
+ * test when targeting a set of HostAndPorts to hedge an operation, so tests
+ * can make assertions about which hosts receive authoritative vs. hedged requests.
+ */
+bool compareByLowerHostThenPort(const HostAndPort& a, const HostAndPort& b);
 }  // namespace mongo
diff --git a/src/mongo/executor/hedge_options_util_test.cpp b/src/mongo/executor/hedge_options_util_test.cpp
index 174498da803..f4d53650dc2 100644
--- a/src/mongo/executor/hedge_options_util_test.cpp
+++ b/src/mongo/executor/hedge_options_util_test.cpp
@@ -184,5 +184,28 @@ TEST_F(HedgeOptionsUtilTestFixture, MaxTimeMSForHedgedReads) {
     checkHedgeOptions(parameters, cmdObj, rspObj, true, 100);
 }
 
+TEST(HedgeOptionsUtilTest, HostAndPortSorting) {
+    struct Spec {
+        HostAndPort a;
+        HostAndPort b;
+        bool out;
+    };
+    static const Spec specs[]{
+        {HostAndPort("ABC"), HostAndPort("abb"), false},
+        {HostAndPort("abc"), HostAndPort("abcd"), true},
+        {HostAndPort("abc", 1), HostAndPort("abc", 2), true},
+        {HostAndPort("de:35"), HostAndPort("de:32"), false},
+        {HostAndPort("def:34"), HostAndPort("dEF:39"), true},
+        {HostAndPort("[0:0:0]"), HostAndPort("abc"), true},
+        {HostAndPort("[0:0:0]"), HostAndPort("ABC"), true},
+    };
+    for (const auto& [a, b, out] : specs) {
+        ASSERT_EQ(compareByLowerHostThenPort(a, b), out) << ", a:" << a << ", b:" << b;
+        if (out) {
+            ASSERT_EQ(compareByLowerHostThenPort(b, a), !out) << ", a:" << a << ", b:" << b;
+        }
+    }
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/executor/hedged_async_rpc.h b/src/mongo/executor/hedged_async_rpc.h
index 442b889c017..417e609f3f8 100644
--- a/src/mongo/executor/hedged_async_rpc.h
+++ b/src/mongo/executor/hedged_async_rpc.h
@@ -156,6 +156,16 @@ SemiFuture<AsyncRPCResponse<typename CommandType::Reply>> sendHedgedCommand(
                 // We'll send 1 authoritative command + however many hedges we can.
                 size_t hostsToTarget = std::min(opts.hedgeCount + 1, targets.size());
 
+                bool targetHostsInAlphabeticalOrder = MONGO_unlikely(
+                    hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder.shouldFail(
+                        [&](const BSONObj&) { return opts.isHedgeEnabled; }));
+
+                if (targetHostsInAlphabeticalOrder) {
+                    std::sort(targets.begin(), targets.end(), [](auto&& a, auto&& b) {
+                        return compareByLowerHostThenPort(a, b);
+                    });
+                }
+
                 for (size_t i = 0; i < hostsToTarget; i++) {
                     std::unique_ptr<Targeter> t = std::make_unique<FixedTargeter>(targets[i]);
                     // We explicitly pass "NeverRetryPolicy" here because the retry mechanism
diff --git a/src/mongo/executor/network_interface.cpp b/src/mongo/executor/network_interface.cpp
index 3fd4b77e0b8..132edf62e02 100644
--- a/src/mongo/executor/network_interface.cpp
+++ b/src/mongo/executor/network_interface.cpp
@@ -38,7 +38,6 @@ namespace executor {
 NetworkInterface::NetworkInterface() {}
 NetworkInterface::~NetworkInterface() {}
 
-MONGO_FAIL_POINT_DEFINE(networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder);
 MONGO_FAIL_POINT_DEFINE(networkInterfaceDiscardCommandsBeforeAcquireConn);
 MONGO_FAIL_POINT_DEFINE(networkInterfaceHangCommandsAfterAcquireConn);
 MONGO_FAIL_POINT_DEFINE(networkInterfaceCommandsFailedWithErrorCode);
diff --git a/src/mongo/executor/network_interface.h b/src/mongo/executor/network_interface.h
index 799f473a12d..91cd5fd3a64 100644
--- a/src/mongo/executor/network_interface.h
+++ b/src/mongo/executor/network_interface.h
@@ -43,7 +43,6 @@
 namespace mongo {
 namespace executor {
 
-extern FailPoint networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder;
 extern FailPoint networkInterfaceDiscardCommandsBeforeAcquireConn;
 extern FailPoint networkInterfaceHangCommandsAfterAcquireConn;
 extern FailPoint networkInterfaceCommandsFailedWithErrorCode;
diff --git a/src/mongo/executor/network_interface_tl.cpp b/src/mongo/executor/network_interface_tl.cpp
index c15a6352be5..c1b3def21ef 100644
--- a/src/mongo/executor/network_interface_tl.cpp
+++ b/src/mongo/executor/network_interface_tl.cpp
@@ -40,6 +40,7 @@
 #include "mongo/db/server_options.h"
 #include "mongo/db/wire_version.h"
 #include "mongo/executor/connection_pool_tl.h"
+#include "mongo/executor/hedge_options_util.h"
 #include "mongo/executor/hedging_metrics.h"
 #include "mongo/executor/network_interface_tl_gen.h"
 #include "mongo/logv2/log.h"
@@ -605,12 +606,12 @@ Status NetworkInterfaceTL::startCommand(const TaskExecutor::CallbackHandle& cbHa
     }
 
     bool targetHostsInAlphabeticalOrder =
-        MONGO_unlikely(networkInterfaceSendRequestsToTargetHostsInAlphabeticalOrder.shouldFail(
+        MONGO_unlikely(hedgedReadsSendRequestsToTargetHostsInAlphabeticalOrder.shouldFail(
             [&](const BSONObj&) { return request.options.hedgeOptions.isHedgeEnabled; }));
 
     if (targetHostsInAlphabeticalOrder) {
         std::sort(request.target.begin(), request.target.end(), [](auto&& a, auto&& b) {
-            return detail::orderByLowerHostThenPort(a, b);
+            return compareByLowerHostThenPort(a, b);
         });
     }
 
@@ -1464,18 +1465,5 @@ bool NetworkInterfaceTL::onNetworkThread() {
 void NetworkInterfaceTL::dropConnections(const HostAndPort& hostAndPort) {
     _pool->dropConnections(hostAndPort);
 }
-
-namespace detail {
-
-bool orderByLowerHostThenPort(const HostAndPort& a, const HostAndPort& b) {
-    const auto& ah = a.host();
-    const auto& bh = b.host();
-    if (int r = compareTransformed(
-            ah.begin(), ah.end(), bh.begin(), bh.end(), [](auto&& c) { return ctype::toLower(c); }))
-        return r < 0;
-    return a.port() < b.port();
-}
-
-}  // namespace detail
 }  // namespace executor
 }  // namespace mongo
diff --git a/src/mongo/executor/network_interface_tl.h b/src/mongo/executor/network_interface_tl.h
index ca0cb1b8a72..a9351ca689f 100644
--- a/src/mongo/executor/network_interface_tl.h
+++ b/src/mongo/executor/network_interface_tl.h
@@ -417,12 +417,5 @@ private:
     stdx::condition_variable _workReadyCond;
     bool _isExecutorRunnable = false;
 };
-
-namespace detail {
-
-/** Order HostAndPorts by their lowercased hostnames then port number */
-bool orderByLowerHostThenPort(const HostAndPort& a, const HostAndPort& b);
-
-}  // namespace detail
 }  // namespace executor
 }  // namespace mongo
diff --git a/src/mongo/executor/network_interface_tl_test.cpp b/src/mongo/executor/network_interface_tl_test.cpp
deleted file mode 100644
index 06bafcbafe3..00000000000
--- a/src/mongo/executor/network_interface_tl_test.cpp
+++ /dev/null
@@ -1,64 +0,0 @@
-/**
- *    Copyright (C) 2022-present MongoDB, Inc.
- *
- *    This program is free software: you can redistribute it and/or modify
- *    it under the terms of the Server Side Public License, version 1,
- *    as published by MongoDB, Inc.
- *
- *    This program is distributed in the hope that it will be useful,
- *    but WITHOUT ANY WARRANTY; without even the implied warranty of
- *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *    Server Side Public License for more details.
- *
- *    You should have received a copy of the Server Side Public License
- *    along with this program. If not, see
- *    <http://www.mongodb.com/licensing/server-side-public-license>.
- *
- *    As a special exception, the copyright holders give permission to link the
- *    code of portions of this program with the OpenSSL library under certain
- *    conditions as described in each individual source file and distribute
- *    linked combinations including the program with the OpenSSL library. You
- *    must comply with the Server Side Public License in all respects for
- *    all of the code used other than as permitted herein. If you modify file(s)
- *    with this exception, you may extend this exception to your version of the
- *    file(s), but you are not obligated to do so. If you do not wish to do so,
- *    delete this exception statement from your version. If you delete this
- *    exception statement from all source files in the program, then also delete
- *    it in the license file.
- */
-
-
-#include "mongo/executor/network_interface_tl.h"
-#include "mongo/unittest/unittest.h"
-#include "mongo/util/net/hostandport.h"
-
-namespace mongo {
-namespace executor {
-namespace {
-
-TEST(NetworkInterfaceTL, HostAndPortSorting) {
-    struct Spec {
-        HostAndPort a;
-        HostAndPort b;
-        bool out;
-    };
-    static const Spec specs[]{
-        {HostAndPort("ABC"), HostAndPort("abb"), false},
-        {HostAndPort("abc"), HostAndPort("abcd"), true},
-        {HostAndPort("abc", 1), HostAndPort("abc", 2), true},
-        {HostAndPort("de:35"), HostAndPort("de:32"), false},
-        {HostAndPort("def:34"), HostAndPort("dEF:39"), true},
-        {HostAndPort("[0:0:0]"), HostAndPort("abc"), true},
-        {HostAndPort("[0:0:0]"), HostAndPort("ABC"), true},
-    };
-    for (const auto& [a, b, out] : specs) {
-        ASSERT_EQ(detail::orderByLowerHostThenPort(a, b), out) << ", a:" << a << ", b:" << b;
-        if (out) {
-            ASSERT_EQ(detail::orderByLowerHostThenPort(b, a), !out) << ", a:" << a << ", b:" << b;
-        }
-    }
-}
-
-}  // namespace
-}  // namespace executor
-}  // namespace mongo