CLDSRV-968: Expire idle keep-alive sockets in the functional test client - #6247
CLDSRV-968: Expire idle keep-alive sockets in the functional test client#6247tcarmet wants to merge 1 commit into
Conversation
The SDK agents in the aws-node-sdk test client enable keepAlive without an idle
timeout, so pooled sockets are never expired client-side. Node's HTTP server
closes idle keep-alive connections after 5s and advertises `Keep-Alive:
timeout=5`, but the agent only honours that hint when it already has a non-zero
timeout of its own. Without one, a burst of requests issued after an idle period
can be written to connections the server has already closed, failing with
ECONNRESET ("socket hang up").
Set a 4s agent idle timeout so the client retires sockets before the server
does. This only destroys sockets sitting in the free pool; in-flight requests
stay governed by requestTimeout.
Hello tcarmet,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
| maxSockets: 200, | ||
| keepAlive: true, | ||
| keepAliveMsecs: 1000, | ||
| timeout: AGENT_IDLE_TIMEOUT, |
There was a problem hiding this comment.
FWIW, nodejs 22.20 has a new option agentKeepAliveTimeoutBuffer of 1 second by default, which would naturally have the same effect here but in a more perennial way (https://nodejs.org/docs/latest/api/http.html#new-agentoptions). Currently the node version is pinned to 22.14 so it's not present, I think the proposed fix is good in the meantime but we could consider reverting it in the future (maybe worth a TODO)
There was a problem hiding this comment.
Oh nice! Definitely worth a todo yeah. I'll create a ticket for nodejs upgrade add a todo in the comments to remove this config when tackling it.
5 participants
The functional test client keeps pooled keep-alive connections indefinitely, so after an idle gap it can send a request over a connection the server has already closed and fail with a spurious connection reset. Because the client also runs with retries disabled, one reset is enough to fail a hook and take a whole group of tests down with it, which surfaces as intermittent CI red on slower full-deployment environments where such idle gaps actually occur.
This makes the client expire its own idle connections before the server does, so the reuse race cannot arise.