Consider maxSockets limits to avoid common pitfalls

[zamnuts]

Currently, maxSockets: Infinity (which is actually Node.js' default anyway) is declared and passed to teeny-request within util:

nodejs-common/src/util.ts

Lines 37 to 40 in 13ae85c

	forever: true,
	pool: {
	maxSockets: Infinity,
	},

During issue triage for googleapis/nodejs-bigquery#624 it was found that the reference implementation from the OP did not take into account the async nature of the API call, and subsequently caused 40,000 concurrent HTTPS (i.e. TLSSockets) to be opened at once. This caused timeouts, ephemeral source port exhaustion, and unnecessarily large RAM usage (more details in the linked issue). This is a common mistake, and the resulting errors are obscure and don't readily indicate root cause, making it difficult to troubleshoot.

I propose 2 options to combat the common pitfall:

1. monitor teeny-request's concurrent socket count, and start emitting warnings when it gets to be abnormally large (depends on maxSockets is not passed to the underlying agent when forever:true teeny-request#148)
2. include an internal buffer to artificially queue/throttle the number of concurrent connections (globally or per API)

As another more practical case, during an incoming request burst (e.g. in a web app) for which perhaps multiple GCP API calls are made in parallel to respond to the request, the number of outbound sockets will be a multiple of incoming sockets. While this is an indicator that the application needs to scale, any solution so this problem will also affect this scenario - perhaps negatively if outbound requests are throttled.

cc: @bcoe @JustinBeckwith

[stephenplusplus]

I like both of those options! Warnings would be helpful, akin to the famous EventEmitter max listeners warning. If #­2 is implementable, it would be nice to do as an opt-in option, unless everybody thinks that's safe to do by default without causing any surprises for users.

[bcoe]

I'm not a huge fan of 2., I think this level of resource management should be happening at the Node.js or operating system level, e.g., I'd rather not implement back pressure in teeny-request.

I like the idea of 1., I think we could pick a reasonable threshold where it seems like we should be warning, e.g., 512 concurrent connections, usually you'd want to limit the number of parallel requests to something below a threshold (IMO).

[zamnuts]

we could pick a reasonable threshold where it seems like we should be warning, e.g., 512 concurrent connections

This is the problem though: what is a reasonable threshold to warn? In the case of a single instance of an application capable of sustaining 2000 TPS, assuming RTT at ~500ms, 512 would not be a big enough number. On the contrary, a serverless function may only require about 50-100 for the threshold. Whatever is chosen will be highly opinionated about the architectural decisions for the software, but at least this won't be a hard ceiling.

---

Warnings would be helpful, akin to the famous EventEmitter max listeners warning

Correct, this would be the effect. Most importantly though is the reason behind the max listeners warning: programmatically detecting code smell. There are (rare) cases where the max listeners threshold in Node.js is too low, the implementer makes a conscious decision about this and subsequently increases the threshold, or rearchitects the implementation.

---

A few thousand (2-5k) outbound requests would be a safe bet IMO: high enough to not be noisy, but low enough to detect the pitfall before things get out of control. This warning would surface during load/performance testing, or somewhat early in the life of an application.

Additionally, an easy way to suppress or change this threshold would provide an opt-out or tuning feature. See EventEmitter.defaultMaxListeners.

[zamnuts]

Decision: warn on threshold, should be configurable and the ability to disable; pick a sane upper bound.