Description
- Version: v13.11.0+
- Platform:
- Subsystem: http
We have a desire to limit the overall lifetime of a socket used for http keep-alive, we've done this with a simple (possibly non-ideal) agent implementation that looks similar to below:
Expand to see HttpAgent implementation
'use strict';
const _HttpsAgent = require('https').Agent;
const MAX_SOCKET_LIFETIME_MS = 10 * 60 * 1000;
const MAX_KEEPALIVE_IDLE_MS = 50 * 1000;
// By default timeouts on a socket don't really do anything. As such, after
// setting a timeout when a socket is kept for keep-alive, it will still get
// re-used again after the timeout period unless it is closed by other means.
// Listening for the timeout event lets use close the socket and remove it from
// the pool. This listener is added when sockets get kept, and removed when
// they get re-used, so should have no risk of effecting external behaviour.
function onTimeout() {
this.destroy();
}
function setSocketCreationTime(err, socket) {
if (err) return;
socket._creation = Date.now();
}
function socketIsExpired(agent, socket) {
if (!socket._creation) {
// shouldn't happen, but covering my bases
setSocketCreationTime(null, socket);
return false;
}
return Date.now() - socket._creation > agent.maxSocketLifetime;
}
function setSocketTimeout(agent, socket) {
const ttl = socket._creation - Date.now() + agent.maxSocketLifetime;
const timeout = Math.min(ttl, agent.maxIdleTimeout);
socket.setTimeout(timeout, onTimeout);
}
function clearSocketTimeout(socket) {
socket.setTimeout(0, onTimeout);
}
class HttpsAgent extends _HttpsAgent {
constructor(opts) {
opts = opts || {};
opts.keepAlive = opts.keepAlive !== false;
super(opts);
this.maxSocketLifetime = MAX_SOCKET_LIFETIME_MS;
this.maxIdleTimeout = MAX_KEEPALIVE_IDLE_MS;
}
createSocket(req, options, cb) {
return super.createSocket(req, options, (err, socket) => {
setSocketCreationTime(err, socket);
return cb(err, socket);
});
}
keepSocketAlive(socket) {
// while we want keep-alive for latency/performance, we don't want to
// keep sockets around forever so that if a backend moves (without
// necessarily closing connections) we will eventually hit the new
// addresses
if (socketIsExpired(this, socket)) {
// returning false means the agent will destroy the socket instead
// of keeping it for re-use
return false;
}
// by default sockets will never time out
// set a timeout for our remaining desired lifetime of the socket, so
// if it doesn't get re-used before then, it will be destroyed for
// being too old while sitting idle
setSocketTimeout(this, socket);
return super.keepSocketAlive(socket);
}
reuseSocket(socket, req) {
// we set a timeout when this socket last went back into the pool so
// that it didnt sit idle past its desired lifetime; however that could
// have been as little as say, 100ms. We don't want to cause
// inconsistent behaviour, having the socket time out during a "slow"
// request to the backend, so reset the timeout back to infinity.
// this can still be set by request.timeout() afterward, as normal
clearSocketTimeout(socket);
return super.reuseSocket(socket, req);
}
}The overall approach:
- Track when the socket was created
- Check if its passed its lifetime when entering the freelist and reject it
- Set a timeout for its remaining lifetime (or a "max free time") when entering the freelist
Which has served us fairly well, and I believe similar mechanisms exist in more featureful "keep-alive agent" node modules.
The built-in agent started trying to handle some of this behaviour in 13.11 (8700d89#diff-5f7fb0850412c6be189faeddea6c5359 #32000) by setting its own timeout listener (removing the need for our agent's onTimeout), but also by resetting the socket's timeout just before re-inserting it into the freelist. Unfortunately this setTimeout proceeds the call to keepSocketAlive, overriding the behaviour we've implemented.
} else if (this.keepSocketAlive(socket)) {
freeSockets = freeSockets || [];
this.freeSockets[name] = freeSockets;
socket[async_id_symbol] = -1;
socket._httpMessage = null;
this.removeSocket(socket, options);
+ const agentTimeout = this.options.timeout || 0;
+ if (socket.timeout !== agentTimeout) {
+ socket.setTimeout(agentTimeout);
+ }
+
freeSockets.push(socket);The documentation on the agent's overridable hooks doesn't necessarily describe what may happen before or after them, but this is something we'd like to be able to maintain.
I may be able to follow up with a PR in the next few days. My first thought would be to move the agent's built-in timeout reset be moved into the default keepSocketAlive function.
Thanks!