Skip to content

CLDSRV-968: Expire idle keep-alive sockets in the functional test client #6247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tcarmet wants to merge 1 commit into
base: development/9.3
Choose a base branch
from
+14 −0
Open

CLDSRV-968: Expire idle keep-alive sockets in the functional test client #6247

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions tests/functional/aws-node-sdk/test/support/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ const DEFAULT_GLOBAL_OPTIONS = {
// timeout.
const REQUEST_TIMEOUT = 30000;

// Idle timeout for pooled keep-alive sockets, which must stay below the

@BourgoisMickael BourgoisMickael Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment could be more succinct

// server's own keep-alive timeout. Node's HTTP server closes idle keep-alive
// connections after 5s and advertises that as `Keep-Alive: timeout=5`, but the
// agent only acts on the hint when it already has a non-zero timeout of its own
// (`agentTimeout = this.options.timeout || 0`, and the hint can only lower an
// existing value). Without one, the agent keeps pooled sockets forever and
// eventually writes to connections the server has already closed, which
// surfaces as ECONNRESET ("socket hang up"). This only ever destroys sockets
// sitting in the free pool: an in-flight request is governed by
// REQUEST_TIMEOUT, which the SDK applies to the socket for its duration.
const AGENT_IDLE_TIMEOUT = 4000;

const DEFAULT_MEM_OPTIONS = {
endpoint: `${transport}://127.0.0.1:8000`,
port: 8000,
Expand All @@ -43,6 +55,7 @@ const DEFAULT_MEM_OPTIONS = {
maxSockets: 200,
keepAlive: true,
keepAliveMsecs: 1000,
timeout: AGENT_IDLE_TIMEOUT,

@jonathan-gramain jonathan-gramain Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@tcarmet tcarmet Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}),
}),
};
Expand All @@ -57,6 +70,7 @@ const DEFAULT_AWS_OPTIONS = {
maxSockets: 200,
keepAlive: true,
keepAliveMsecs: 1000,
timeout: AGENT_IDLE_TIMEOUT,
}),
}),
};
Expand Down
Loading