SERVER-105753 Ensure connection establishment rate limiter UX consistency with operation rate limiter (#36793)

GitOrigin-RevId: 01486ee5ef0c7f56f1ef53b04f039b9cd4270f96

Author: erin2722

jstests/noPassthrough/connection_establishment_rate_limiting.js

@@ -51,6 +51,7 @@ const testKillOnClientDisconnect = (conn) => {
 };
 
 const testKillOnClientDisconnectOpts = {
+    ingressConnectionEstablishmentRateLimiterEnabled: true,
     ingressConnectionEstablishmentRatePerSec: 1,
     ingressConnectionEstablishmentBurstSize: 1,
     ingressConnectionEstablishmentMaxQueueDepth: maxQueueSize,

jstests/noPassthrough/connection_establishment_rate_limiting_client_disconnect.js

@@ -48,10 +48,11 @@ const testExemptIPsFromRateLimit = (conn) => {
 };
 
 const testExemptIPsFromRateLimitOpts = {
+    ingressConnectionEstablishmentRateLimiterEnabled: true,
     ingressConnectionEstablishmentRatePerSec: 1,
     ingressConnectionEstablishmentBurstSize: 1,
     ingressConnectionEstablishmentMaxQueueDepth: 0,
-    maxEstablishingConnectionsOverride: {ranges: [exemptIP]},
+    ingressConnectionEstablishmentRateLimiterBypass: {ranges: [exemptIP]},
 };
 runTestStandaloneParamsSetAtStartup(testExemptIPsFromRateLimitOpts, testExemptIPsFromRateLimit);
 runTestStandaloneParamsSetAtRuntime(testExemptIPsFromRateLimitOpts, testExemptIPsFromRateLimit);

jstests/noPassthrough/connection_establishment_rate_limiting_exemptions.js

@@ -32,10 +32,11 @@ let rs = new ReplSetTest({
         "proxyPort": egressPort,
         config: "jstests/noPassthrough/libs/max_conns_override_config.yaml",
         setParameter: {
+            ingressConnectionEstablishmentRateLimiterEnabled: true,
             ingressConnectionEstablishmentRatePerSec: 1,
             ingressConnectionEstablishmentBurstSize: 1,
             ingressConnectionEstablishmentMaxQueueDepth: 0,
-            maxEstablishingConnectionsOverride: {ranges: [exemptIP]},
+            ingressConnectionEstablishmentRateLimiterBypass: {ranges: [exemptIP]},
             featureFlagRateLimitIngressConnectionEstablishment: true
         }
     }
@@ -67,8 +68,7 @@ rs.initiate();
 // Let connections through again.
 rs.getPrimary().adminCommand({
     setParameter: 1,
-    ingressConnectionEstablishmentRatePerSec: 10,
-    ingressConnectionEstablishmentBurstSize: 500,
+    ingressConnectionEstablishmentRateLimiterEnabled: false,
 });
 
 // Start up a proxy protocol server with an exempt IP as its egress address. Ensure that non-exempt
@@ -81,8 +81,7 @@ rs.getPrimary().adminCommand({
     // Reset the rate limiter to use lower values again.
     rs.getPrimary().adminCommand({
         setParameter: 1,
-        ingressConnectionEstablishmentRatePerSec: 1,
-        ingressConnectionEstablishmentBurstSize: 1,
+        ingressConnectionEstablishmentRateLimiterEnabled: true,
     });
 
     // One token will be consumed by a non-exempt IP.

jstests/noPassthrough/connection_establishment_rate_limiting_proxy_protocol.js

@@ -93,6 +93,7 @@ const testRateLimiterStats = (conn) => {
 };
 
 const testRateLimiterStatsOpts = {
+    ingressConnectionEstablishmentRateLimiterEnabled: true,
     ingressConnectionEstablishmentRatePerSec: 1,
     ingressConnectionEstablishmentBurstSize: 1,
     ingressConnectionEstablishmentMaxQueueDepth: maxQueueSize,

jstests/noPassthrough/connection_establishment_rate_limiting_stats.js

@@ -1,8 +1,8 @@
 // Tests using a server parameter to set `maxIncomingConnectionsOverride` and
-// `maxEstablishingConnectionsOverride` at runtime.
+// `ingressConnectionEstablishmentRateLimiterBypass` at runtime.
 
 const maxIncoming = "maxIncomingConnectionsOverride";
-const maxEstablishing = "maxEstablishingConnectionsOverride";
+const maxEstablishing = "ingressConnectionEstablishmentRateLimiterBypass";
 
 function runTest(args, testFunc) {
     // Run tests in isolation to make sure we always start with a clean slate.

jstests/noPassthrough/max_conns_override_runtime.js

@@ -65,14 +65,6 @@ SessionEstablishmentRateLimiter* SessionEstablishmentRateLimiter::get(ServiceCon
 
 
 Status SessionEstablishmentRateLimiter::throttleIfNeeded(Client* client) {
-    // We can short-circuit if the rate limit is unlimited, which probably means the feature
-    // is off.
-    if (gIngressConnectionEstablishmentRatePerSec.loadRelaxed() ==
-        std::numeric_limits<int>::max()) {
-        // Note that in this case, no metrics are maintained.
-        return Status::OK();
-    }
-
     // Check if the session is exempt from rate limiting based on its IP.
     serverGlobalParams.maxEstablishingConnsOverride.refreshSnapshot(_maxEstablishingConnsOverride);
     if (_maxEstablishingConnsOverride &&

src/mongo/transport/session_establishment_rate_limiter.cpp

@@ -80,6 +80,7 @@
 #include "mongo/transport/session_manager.h"
 #include "mongo/transport/session_workflow.h"
 #include "mongo/transport/transport_layer_manager.h"
+#include "mongo/transport/transport_options_gen.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/clock_source.h"
 #include "mongo/util/concurrency/idle_thread_block.h"
@@ -817,7 +818,8 @@ void SessionWorkflow::Impl::_scheduleIteration() try {
                 const auto fcvSnapshot =
                     serverGlobalParams.featureCompatibility.acquireFCVSnapshot();
                 if (gFeatureFlagRateLimitIngressConnectionEstablishment
-                        .isEnabledUseLatestFCVWhenUninitialized(fcvSnapshot)) {
+                        .isEnabledUseLatestFCVWhenUninitialized(fcvSnapshot) &&
+                    gIngressConnectionEstablishmentRateLimiterEnabled.load()) {
                     uassertStatusOK(session()
                                         ->getTransportLayer()
                                         ->getSessionManager()

src/mongo/transport/session_workflow.cpp

@@ -606,6 +606,8 @@ class ConnectionEstablishmentQueueingTest : public SessionWorkflowTest {
 
     RAIIServerParameterControllerForTest featureFlagController{
         "featureFlagRateLimitIngressConnectionEstablishment", true};
+    RAIIServerParameterControllerForTest featureEnabled{
+        "ingressConnectionEstablishmentRateLimiterEnabled", true};
     unittest::MinimumLoggedSeverityGuard logSeverityGuard{logv2::LogComponent::kDefault,
                                                           logv2::LogSeverity::Debug(4)};
 };
@@ -680,7 +682,8 @@ TEST_F(ConnectionEstablishmentQueueingTest, InterruptQueuedEstablishments) {
 TEST_F(ConnectionEstablishmentQueueingTest, BypassQueueingEstablishment) {
     std::string ip = "127.0.0.1";
     RAIIServerParameterControllerForTest exemptionsGuard(
-        "maxEstablishingConnectionsOverride", BSON("ranges" << BSONArray(BSON("0" << ip))));
+        "ingressConnectionEstablishmentRateLimiterBypass",
+        BSON("ranges" << BSONArray(BSON("0" << ip))));
     RAIIServerParameterControllerForTest refreshRate{"ingressConnectionEstablishmentRatePerSec",
                                                      1.0};
     RAIIServerParameterControllerForTest burstSize{"ingressConnectionEstablishmentBurstSize", 1};

src/mongo/transport/session_workflow_test.cpp

@@ -81,17 +81,25 @@ server_parameters:
         override_set: true
     redact: false
 
-  maxEstablishingConnectionsOverride:
-    description: "A list of CIDRs to be exempted from the max establishing limits"
+  ingressConnectionEstablishmentRateLimiterBypass:
+    description: "A list of CIDRs to be exempted from the ingress connection establishment rate limit"
     set_at: [startup, runtime]
     cpp_class:
         name: "MaxEstablishingConnectionsOverrideServerParameter"
         # Expects the payload to be an instance of `ConnectionListParameters`.
         override_set: true
     redact: false
-  
+
+  ingressConnectionEstablishmentRateLimiterEnabled:
+    description: "Whether the ingress connection establishment rate limiter is enabled or not"
+    set_at: [startup, runtime]
+    cpp_varname: gIngressConnectionEstablishmentRateLimiterEnabled
+    cpp_vartype: Atomic<bool>
+    default: false
+    redact: false
+
   ingressConnectionEstablishmentRatePerSec:
-    description: "The number of new connections that will be allowed to establish per second"
+    description: "The number of new ingress connections that will be allowed to establish per second"
     set_at: [startup, runtime]
     cpp_varname: gIngressConnectionEstablishmentRatePerSec
     cpp_vartype: Atomic<int32_t>
@@ -103,7 +111,7 @@ server_parameters:
 
   ingressConnectionEstablishmentBurstSize:
     description: >-
-        The maximum number of connection establishments that will be admitted before rate-limiting 
+        The maximum number of ingress connection establishments that will be admitted before rate-limiting 
         kicks in (ie, the burst size of the bucket)
     set_at: [startup, runtime]
     cpp_varname: gIngressConnectionEstablishmentBurstSize
@@ -117,8 +125,8 @@ server_parameters:
   ingressConnectionEstablishmentMaxQueueDepth:
     description: >-
         The maximum size of the connection establishment queue, after which the server will begin 
-        rejecting new connections. A queue size of 0 indicates that connections will be rejected if 
-        they cannot immediately be admitted.
+        rejecting new ingress connections. A queue size of 0 indicates that connections will be 
+        rejected if they cannot immediately be admitted.
     set_at: [startup, runtime]
     cpp_varname: gIngressConnectionEstablishmentMaxQueueDepth
     cpp_vartype: Atomic<int32_t>