Connection Manager Acquisition Timeout (#479)

Co-authored-by: Dengke Tang <[email protected]>
Co-authored-by: Michael Graeb <[email protected]>

Author: 3 people

include/aws/http/connection_manager.h

@@ -125,6 +125,13 @@ struct aws_http_connection_manager_options {
      */
     uint64_t max_connection_idle_in_milliseconds;
 
+    /**
+     * If set to a non-zero value, aws_http_connection_manager_acquire_connection() calls
+     * will give up after waiting this long for a connection from the pool,
+     * failing with error AWS_ERROR_HTTP_CONNECTION_MANAGER_ACQUISITION_TIMEOUT.
+     */
+    uint64_t connection_acquisition_timeout_ms;
+
     /**
      * THIS IS AN EXPERIMENTAL AND UNSTABLE API
      * (Optional)

include/aws/http/http.h

@@ -60,6 +60,7 @@ enum aws_http_errors {
     AWS_ERROR_HTTP_MANUAL_WRITE_NOT_ENABLED,
     AWS_ERROR_HTTP_MANUAL_WRITE_HAS_COMPLETED,
     AWS_ERROR_HTTP_RESPONSE_FIRST_BYTE_TIMEOUT,
+    AWS_ERROR_HTTP_CONNECTION_MANAGER_ACQUISITION_TIMEOUT,
 
     AWS_ERROR_HTTP_END_RANGE = AWS_ERROR_ENUM_END_RANGE(AWS_C_HTTP_PACKAGE_ID)
 };

source/connection_manager.c

@@ -193,7 +193,10 @@ struct aws_http_connection_manager {
     struct aws_linked_list idle_connections;
 
     /*
-     * The set of all incomplete connection acquisition requests
+     * The set of all incomplete connection acquisition requests.
+     * This must be a FIFO list. When connections are requested by the user, they are added to the back. When we need to
+     * complete the acquisition, we pop from the front. In this way, the list is always sorted from the oldest (in terms
+     * of timeout timestamp) to the newest and we can cull it similar to idle_connections.
      */
     struct aws_linked_list pending_acquisitions;
 
@@ -286,6 +289,8 @@ struct aws_http_connection_manager {
      */
     uint64_t max_connection_idle_in_milliseconds;
 
+    uint64_t connection_acquisition_timeout_ms;
+
     /*
      * Task to cull idle connections.  This task is run periodically on the cull_event_loop if a non-zero
      * culling time interval is specified.
@@ -392,6 +397,7 @@ struct aws_http_connection_acquisition {
     struct aws_http_connection *connection;
     int error_code;
     struct aws_channel_task acquisition_task;
+    uint64_t timeout_timestamp;
 };
 
 static void s_connection_acquisition_task(
@@ -758,26 +764,17 @@ static void s_final_destruction_task(struct aws_task *task, void *arg, enum aws_
 }
 
 static void s_cull_task(struct aws_task *task, void *arg, enum aws_task_status status);
-static void s_schedule_connection_culling(struct aws_http_connection_manager *manager) {
-    if (manager->max_connection_idle_in_milliseconds == 0) {
-        return;
-    }
-
-    if (manager->cull_task == NULL) {
-        manager->cull_task = aws_mem_calloc(manager->allocator, 1, sizeof(struct aws_task));
-        aws_task_init(manager->cull_task, s_cull_task, manager, "cull_idle_connections");
-        /* For the task to properly run and cancel, we need to keep manager alive */
-        aws_ref_count_acquire(&manager->internal_ref_count);
-    }
 
-    if (manager->cull_event_loop == NULL) {
-        manager->cull_event_loop = aws_event_loop_group_get_next_loop(manager->bootstrap->event_loop_group);
+/*
+ * Calculates the next timestamp the idle connections should be culled. Manager lock must be held somewhere in the call
+ * stack. Returns UINT64_MAX if max_connection_idle_in_milliseconds is not set.
+ */
+static uint64_t s_calculate_idle_connection_cull_task_time_synced(struct aws_http_connection_manager *manager) {
+    if (manager->max_connection_idle_in_milliseconds == 0) {
+        return UINT64_MAX;
     }
-    AWS_FATAL_ASSERT(manager->cull_event_loop != NULL);
-
     uint64_t cull_task_time = 0;
 
-    aws_mutex_lock(&manager->lock);
     const struct aws_linked_list_node *end = aws_linked_list_end(&manager->idle_connections);
     struct aws_linked_list_node *oldest_node = aws_linked_list_begin(&manager->idle_connections);
     if (oldest_node != end) {
@@ -799,11 +796,68 @@ static void s_schedule_connection_culling(struct aws_http_connection_manager *ma
             now + aws_timestamp_convert(
                       manager->max_connection_idle_in_milliseconds, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL);
     }
-    aws_mutex_unlock(&manager->lock);
 
-    aws_event_loop_schedule_task_future(manager->cull_event_loop, manager->cull_task, cull_task_time);
+    return cull_task_time;
+}
+
+/*
+ * Calculates the next timestamp the pending acquisitions should be culled. Manager lock must be held somewhere in the
+ * call stack. Returns UINT64_MAX if connection_acquisition_timeout_ms is not set.
+ */
+static uint64_t s_calculate_pending_acquisition_cull_task_time_synced(struct aws_http_connection_manager *manager) {
+    if (manager->connection_acquisition_timeout_ms == 0) {
+        return UINT64_MAX;
+    }
 
-    return;
+    uint64_t cull_task_time = 0;
+
+    const struct aws_linked_list_node *end = aws_linked_list_end(&manager->pending_acquisitions);
+    struct aws_linked_list_node *oldest_node = aws_linked_list_begin(&manager->pending_acquisitions);
+    if (oldest_node != end) {
+        /*
+         * front of the list has the closest cull time
+         */
+        struct aws_http_connection_acquisition *oldest_pending_acquire =
+            AWS_CONTAINER_OF(oldest_node, struct aws_http_connection_acquisition, node);
+        cull_task_time = oldest_pending_acquire->timeout_timestamp;
+    } else {
+        /*
+         * There are no acquisition in the list, so the absolute minimum anything could be culled is the full
+         * culling interval from now.
+         */
+        uint64_t now = 0;
+        manager->system_vtable->aws_high_res_clock_get_ticks(&now);
+        cull_task_time =
+            now + aws_timestamp_convert(
+                      manager->connection_acquisition_timeout_ms, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL);
+    }
+    return cull_task_time;
+}
+
+static void s_schedule_culling(struct aws_http_connection_manager *manager) {
+    if (manager->max_connection_idle_in_milliseconds == 0 && manager->connection_acquisition_timeout_ms == 0) {
+        return;
+    }
+
+    if (manager->cull_task == NULL) {
+        manager->cull_task = aws_mem_calloc(manager->allocator, 1, sizeof(struct aws_task));
+        aws_task_init(manager->cull_task, s_cull_task, manager, "cull_idle_connections");
+        /* For the task to properly run and cancel, we need to keep manager alive */
+        aws_ref_count_acquire(&manager->internal_ref_count);
+    }
+
+    if (manager->cull_event_loop == NULL) {
+        manager->cull_event_loop = aws_event_loop_group_get_next_loop(manager->bootstrap->event_loop_group);
+    }
+    AWS_FATAL_ASSERT(manager->cull_event_loop != NULL);
+
+    aws_mutex_lock(&manager->lock);
+    uint64_t idle_cull_time = s_calculate_idle_connection_cull_task_time_synced(manager);
+    uint64_t acquisition_cull_time = s_calculate_pending_acquisition_cull_task_time_synced(manager);
+    aws_mutex_unlock(&manager->lock);
+
+    aws_event_loop_schedule_task_future(
+        manager->cull_event_loop, manager->cull_task, aws_min_u64(idle_cull_time, acquisition_cull_time));
 }
 
 struct aws_http_connection_manager *aws_http_connection_manager_new(
@@ -900,6 +954,8 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(
     manager->shutdown_complete_user_data = options->shutdown_complete_user_data;
     manager->enable_read_back_pressure = options->enable_read_back_pressure;
     manager->max_connection_idle_in_milliseconds = options->max_connection_idle_in_milliseconds;
+    manager->connection_acquisition_timeout_ms = options->connection_acquisition_timeout_ms;
+
     if (options->proxy_ev_settings) {
         manager->proxy_ev_settings = *options->proxy_ev_settings;
     }
@@ -938,7 +994,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(
     }
 
     /* NOTHING can fail after here */
-    s_schedule_connection_culling(manager);
+    s_schedule_culling(manager);
 
     AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Successfully created", (void *)manager);
 
@@ -1227,6 +1283,22 @@ void aws_http_connection_manager_acquire_connection(
     request->user_data = user_data;
     request->manager = manager;
 
+    if (manager->connection_acquisition_timeout_ms) {
+        uint64_t acquire_start_timestamp = 0;
+        if (manager->system_vtable->aws_high_res_clock_get_ticks(&acquire_start_timestamp) == AWS_OP_SUCCESS) {
+            request->timeout_timestamp =
+                acquire_start_timestamp +
+                aws_timestamp_convert(
+                    manager->connection_acquisition_timeout_ms, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL);
+        } else {
+            AWS_LOGF_WARN(
+                AWS_LS_HTTP_CONNECTION_MANAGER,
+                "id=%p: Failed to get current timestamp using aws_high_res_clock_get_ticks function. Ignoring the "
+                "connection_acquisition_timeout_ms value. ",
+                (void *)manager);
+        }
+    }
+
     struct aws_connection_management_transaction work;
     s_aws_connection_management_transaction_init(&work, manager);
 
@@ -1545,10 +1617,11 @@ static void s_aws_http_connection_manager_on_connection_shutdown(
     s_aws_http_connection_manager_execute_transaction(&work);
 }
 
-static void s_cull_idle_connections(struct aws_http_connection_manager *manager) {
-    AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: culling idle connections", (void *)manager);
+static void s_cull_task_impl(struct aws_http_connection_manager *manager) {
+    AWS_LOGF_INFO(
+        AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: culling idle connections and pending acquisitions", (void *)manager);
 
-    if (manager == NULL || manager->max_connection_idle_in_milliseconds == 0) {
+    if (manager == NULL) {
         return;
     }
 
@@ -1564,26 +1637,53 @@ static void s_cull_idle_connections(struct aws_http_connection_manager *manager)
 
     /* Only if we're not shutting down */
     if (manager->state == AWS_HCMST_READY) {
-        const struct aws_linked_list_node *end = aws_linked_list_end(&manager->idle_connections);
-        struct aws_linked_list_node *current_node = aws_linked_list_begin(&manager->idle_connections);
-        while (current_node != end) {
-            struct aws_linked_list_node *node = current_node;
-            struct aws_idle_connection *current_idle_connection =
-                AWS_CONTAINER_OF(node, struct aws_idle_connection, node);
-            if (current_idle_connection->cull_timestamp > now) {
-                break;
+        /* cull idle connections */
+        if (manager->max_connection_idle_in_milliseconds != 0) {
+            const struct aws_linked_list_node *idle_connections_end = aws_linked_list_end(&manager->idle_connections);
+            struct aws_linked_list_node *idle_connections_current = aws_linked_list_begin(&manager->idle_connections);
+            while (idle_connections_current != idle_connections_end) {
+                struct aws_linked_list_node *node = idle_connections_current;
+                struct aws_idle_connection *current_idle_connection =
+                    AWS_CONTAINER_OF(node, struct aws_idle_connection, node);
+                if (current_idle_connection->cull_timestamp > now) {
+                    break;
+                }
+
+                idle_connections_current = aws_linked_list_next(idle_connections_current);
+                aws_linked_list_remove(node);
+                aws_linked_list_push_back(&work.connections_to_release, node);
+                --manager->idle_connection_count;
+
+                AWS_LOGF_DEBUG(
+                    AWS_LS_HTTP_CONNECTION_MANAGER,
+                    "id=%p: culling idle connection (%p)",
+                    (void *)manager,
+                    (void *)current_idle_connection->connection);
             }
+        }
 
-            current_node = aws_linked_list_next(current_node);
-            aws_linked_list_remove(node);
-            aws_linked_list_push_back(&work.connections_to_release, node);
-            --manager->idle_connection_count;
-
-            AWS_LOGF_DEBUG(
-                AWS_LS_HTTP_CONNECTION_MANAGER,
-                "id=%p: culling idle connection (%p)",
-                (void *)manager,
-                (void *)current_idle_connection->connection);
+        /* cull pending acquisitions */
+        if (manager->connection_acquisition_timeout_ms != 0) {
+            const struct aws_linked_list_node *pending_acquisitions_end =
+                aws_linked_list_end(&manager->pending_acquisitions);
+            struct aws_linked_list_node *pending_acquisitions_current =
+                aws_linked_list_begin(&manager->pending_acquisitions);
+            while (pending_acquisitions_current != pending_acquisitions_end) {
+                struct aws_linked_list_node *node = pending_acquisitions_current;
+                struct aws_http_connection_acquisition *current_pending_acquire =
+                    AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node);
+                if (current_pending_acquire->timeout_timestamp > now) {
+                    break;
+                }
+
+                pending_acquisitions_current = aws_linked_list_next(pending_acquisitions_current);
+                s_aws_http_connection_manager_move_front_acquisition(
+                    manager, NULL, AWS_ERROR_HTTP_CONNECTION_MANAGER_ACQUISITION_TIMEOUT, &work.completions);
+                AWS_LOGF_DEBUG(
+                    AWS_LS_HTTP_CONNECTION_MANAGER,
+                    "id=%p: Failing pending acquisition due to timeout",
+                    (void *)manager);
+            }
         }
     }
 
@@ -1601,10 +1701,9 @@ static void s_cull_task(struct aws_task *task, void *arg, enum aws_task_status s
     }
 
     struct aws_http_connection_manager *manager = arg;
+    s_cull_task_impl(manager);
 
-    s_cull_idle_connections(manager);
-
-    s_schedule_connection_culling(manager);
+    s_schedule_culling(manager);
 }
 
 void aws_http_connection_manager_fetch_metrics(

source/http.c

@@ -151,6 +151,9 @@ static struct aws_error_info s_errors[] = {
     AWS_DEFINE_ERROR_INFO_HTTP(
         AWS_ERROR_HTTP_RESPONSE_FIRST_BYTE_TIMEOUT,
         "The server does not begin responding within the configuration after a request is fully sent."),
+    AWS_DEFINE_ERROR_INFO_HTTP(
+        AWS_ERROR_HTTP_CONNECTION_MANAGER_ACQUISITION_TIMEOUT,
+        "Connection Manager failed to acquire a connection within the defined timeout."),
 };
 /* clang-format on */
 

tests/CMakeLists.txt

@@ -519,6 +519,7 @@ add_net_test_case(connection_customized_alpn_error_with_unknown_return_string)
 # unit tests where connections are mocked
 add_net_test_case(test_connection_manager_setup_shutdown)
 add_net_test_case(test_connection_manager_acquire_release_mix_synchronous)
+add_net_test_case(test_connection_manager_acquisition_timeout)
 add_net_test_case(test_connection_manager_connect_callback_failure)
 add_net_test_case(test_connection_manager_connect_immediate_failure)
 add_net_test_case(test_connection_manager_proxy_setup_shutdown)