Initially, this PR was planned for 1.0.0, but it will be safer to release this as 0.3.0-beta.1 in advance to test this particular changeset.
Resolves #237
Resolves #196 (it's not that KeepAlive was degrading performance; it was that KeepAlive=false was still using the entire KeepAlive mechanism).
Changes (some are breaking):
Idle sockets timeout rework; now, the client tracks the idling sockets (on top of what the KeepAlive agent should normally do) and forcefully destroys them when it sees fit (by default, after 2500ms of idling, to match the default ClickHouse server KeepAlive configuration).
Ensure proper Connection header value considering KeepAlive settings. If KeepAlive is disabled, its value is now forced to "close".
(Breaking) keep_alive.retry_on_expired_socket and keep_alive.socket_ttl configuration parameters are removed. Instead, the new keep_alive.idle_socket_ttl parameter should be used. The default value for keep_alive.idle_socket_ttl is 2500.
Fixed a bug with Ping that could lead to unhandled Socket hang-up propagation.
Added an extra layer of "protection" against potential socket issues to the query/select/exec methods in the underlying HTTP connection implementation. If a request fails, the client will try to ensure that the outgoing request is also destroyed, meaning that the socket will be released immediately; this socket also shouldn't produce any unexpected "results" later (such as, again, unhandled Socket hang up from a rogue socket).
(Technically breaking) The max_open_connections configuration parameter is now 10 by default, as we should not rely on the KeepAlive agent's defaults.
(from 1.0.0, technically breaking) Backported the fix of the default request_timeout configuration value (now it is correctly set to 30s).
Logger and logging improvements - a bit fancier default console writer and more logs on failing requests (if the log level is not OFF in the client configuration (it is off by default)); all client methods except ping will log an error on failure now, and ping will log a warning (cause the error is returned as part of its result).
Split/fixed connection unit tests cause it already had too many test cases for a single file.
Fixed a minor "lint" in toURLSearchParams since query_id has not been optional within the connection for a long time and is present for any request that uses it.
From #234 - removed Node.js 16 from the CI and bumped GHA versions.
A changelog entry will be added if we decide to proceed with this.
Checklist
[x] Unit and integration tests covering the common scenarios were added
[x] A human-readable description of the changes was provided to include in CHANGELOG
Summary
Initially, this PR was planned for 1.0.0, but it will be safer to release this as 0.3.0-beta.1 in advance to test this particular changeset.
Resolves #237 Resolves #196 (it's not that KeepAlive was degrading performance; it was that KeepAlive=false was still using the entire KeepAlive mechanism).
Changes (some are breaking):
Connectionheader value considering KeepAlive settings. If KeepAlive is disabled, its value is now forced to "close".keep_alive.retry_on_expired_socketandkeep_alive.socket_ttlconfiguration parameters are removed. Instead, the newkeep_alive.idle_socket_ttlparameter should be used. The default value forkeep_alive.idle_socket_ttlis 2500.max_open_connectionsconfiguration parameter is now 10 by default, as we should not rely on the KeepAlive agent's defaults.request_timeoutconfiguration value (now it is correctly set to 30s).OFFin the client configuration (it is off by default)); all client methods except ping will log an error on failure now, and ping will log a warning (cause the error is returned as part of its result).toURLSearchParamssince query_id has not been optional within the connection for a long time and is present for any request that uses it.A changelog entry will be added if we decide to proceed with this.
Checklist