Node.js connection idle sockets timeout rework, logger improvements

.github/workflows/tests.yml

@@ -26,12 +26,12 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        node: [16, 18, 20]
+        node: [18, 20, 21]
     steps:
       - uses: actions/checkout@main
 
       - name: Setup NodeJS ${{ matrix.node }}
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node }}
 
@@ -62,17 +62,17 @@ jobs:
       - uses: actions/checkout@main
 
       - name: Start ClickHouse (version - ${{ matrix.clickhouse }}) in Docker
-        uses: isbang/compose-action@v1.1.0
+        uses: isbang/compose-action@v1.5.1
         env:
           CLICKHOUSE_VERSION: ${{ matrix.clickhouse }}
         with:
           compose-file: 'docker-compose.yml'
           down-flags: '--volumes'
 
       - name: Setup NodeJS
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
-          node-version: 16
+          node-version: 20
 
       - name: Install dependencies
         run: |
@@ -88,22 +88,22 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        node: [16, 18, 20]
+        node: [18, 20, 21]
         clickhouse: [head, latest]
 
     steps:
       - uses: actions/checkout@main
 
       - name: Start ClickHouse (version - ${{ matrix.clickhouse }}) in Docker
-        uses: isbang/compose-action@v1.1.0
+        uses: isbang/compose-action@v1.5.1
         env:
           CLICKHOUSE_VERSION: ${{ matrix.clickhouse }}
         with:
           compose-file: 'docker-compose.yml'
           down-flags: '--volumes'
 
       - name: Setup NodeJS ${{ matrix.node }}
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node }}
 
@@ -129,22 +129,22 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        node: [16, 18, 20]
+        node: [18, 20, 21]
         clickhouse: [head, latest]
 
     steps:
       - uses: actions/checkout@main
 
       - name: Start ClickHouse cluster (version - ${{ matrix.clickhouse }}) in Docker
-        uses: isbang/compose-action@v1.1.0
+        uses: isbang/compose-action@v1.5.1
         env:
           CLICKHOUSE_VERSION: ${{ matrix.clickhouse }}
         with:
           compose-file: 'docker-compose.cluster.yml'
           down-flags: '--volumes'
 
       - name: Setup NodeJS ${{ matrix.node }}
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node }}
 
@@ -167,17 +167,17 @@ jobs:
       - uses: actions/checkout@main
 
       - name: Start ClickHouse cluster (version - ${{ matrix.clickhouse }}) in Docker
-        uses: isbang/compose-action@v1.1.0
+        uses: isbang/compose-action@v1.5.1
         env:
           CLICKHOUSE_VERSION: ${{ matrix.clickhouse }}
         with:
           compose-file: 'docker-compose.cluster.yml'
           down-flags: '--volumes'
 
       - name: Setup NodeJS
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
-          node-version: 16
+          node-version: 20
 
       - name: Install dependencies
         run: |
@@ -193,13 +193,13 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        node: [16, 18, 20]
+        node: [18, 20, 21]
 
     steps:
       - uses: actions/checkout@main
 
       - name: Setup NodeJS ${{ matrix.node }}
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node }}
 
@@ -222,9 +222,9 @@ jobs:
       - uses: actions/checkout@main
 
       - name: Setup NodeJS
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
-          node-version: 16
+          node-version: 20
 
       - name: Install dependencies
         run: |

CHANGELOG.md

@@ -1,3 +1,23 @@
+## 0.3.0-beta.1 (Node.js only)
+
+A subset of changes from the upcoming 1.0.0 release.
+
+### New features
+
+- 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).
+- Logging improvements: more logs on failing requests; all client methods except ping will log an error on failure now, and ping will log a warning, since the error is returned as part of its result. Logging needs to be enabled explicitly, as the log level is `OFF` by default.
+
+### Breaking changes
+
+- `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.
+- The `max_open_connections` configuration parameter is now 10 by default, as we should not rely on the KeepAlive agent's defaults.
+- Fixed the default request_timeout configuration value (now it is correctly set to 30s, previously 300s).
+
+### Bug fixes
+
+- Fixed a bug with Ping that could lead to unhandled Socket hang-up propagation.
+- Ensure proper Connection header value considering KeepAlive settings. If KeepAlive is disabled, its value is now forced to ["close"](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection#close).
+
 ## 0.2.10 (Common, Node.js, Web)
 
 ### New features

benchmarks/tsconfig.json

@@ -7,6 +7,7 @@
     "outDir": "dist",
     "baseUrl": "./",
     "paths": {
+      "@clickhouse/client-common": ["../packages/client-common/src/index.ts"],
       "@clickhouse/client": ["../packages/client-node/src/index.ts"],
       "@clickhouse/client/*": ["../packages/client-node/src/*"]
     }

packages/client-common/__tests__/unit/to_search_params.test.ts

@@ -2,18 +2,23 @@ import { toSearchParams } from '@clickhouse/client-common'
 import type { URLSearchParams } from 'url'
 
 describe('toSearchParams', () => {
-  it('should return undefined with default settings', async () => {
-    expect(toSearchParams({ database: 'default' })).toBeUndefined()
+  it('should return only query_id, ignoring the default database', async () => {
+    const params = toSearchParams({ database: 'default', query_id: 'foo' })
+    expect(toSortedArray(params)).toEqual([['query_id', 'foo']])
   })
 
   it('should set database', async () => {
-    const params = toSearchParams({ database: 'mydb' })!
-    expect([...params.entries()]).toEqual([['database', 'mydb']])
+    const params = toSearchParams({ database: 'my_db', query_id: 'foo' })!
+    expect(toSortedArray(params)).toEqual([
+      ['database', 'my_db'],
+      ['query_id', 'foo'],
+    ])
   })
 
   it('should set ClickHouse settings', async () => {
     const params = toSearchParams({
       database: 'default',
+      query_id: 'foo',
       clickhouse_settings: {
         insert_quorum: '2',
         distributed_product_mode: 'global',
@@ -25,12 +30,14 @@ describe('toSearchParams', () => {
       ['distributed_product_mode', 'global'],
       ['insert_quorum', '2'],
       ['limit', '42'],
+      ['query_id', 'foo'],
     ])
   })
 
   it('should set query params', async () => {
     const params = toSearchParams({
       database: 'default',
+      query_id: 'foo',
       query_params: {
         foo: 42,
         bar: true,
@@ -41,16 +48,21 @@ describe('toSearchParams', () => {
       ['param_bar', '1'],
       ['param_foo', '42'],
       ['param_qaz', 'qux'],
+      ['query_id', 'foo'],
     ])
   })
 
   it('should set query', async () => {
     const query = 'SELECT * FROM system.settings'
     const params = toSearchParams({
       database: 'default',
+      query_id: 'foo',
       query,
     })!
-    expect(toSortedArray(params)).toEqual([['query', query]])
+    expect(toSortedArray(params)).toEqual([
+      ['query', query],
+      ['query_id', 'foo'],
+    ])
   })
 
   it('should set everything', async () => {

packages/client-common/src/client.ts

@@ -60,7 +60,7 @@ export interface ClickHouseClientConfigOptions<Stream> {
   host?: string
   /** The request timeout in milliseconds. Default value: `30_000`. */
   request_timeout?: number
-  /** Maximum number of sockets to allow per host. Default value: `Infinity`. */
+  /** Maximum number of sockets to allow per host. Default value: `10`. */
   max_open_connections?: number
 
   compression?: {
@@ -338,8 +338,8 @@ function getConnectionParams<Stream>(
   return {
     application_id: config.application,
     url: createUrl(config.host ?? 'http://localhost:8123'),
-    request_timeout: config.request_timeout ?? 300_000,
-    max_open_connections: config.max_open_connections ?? Infinity,
+    request_timeout: config.request_timeout ?? 30_000,
+    max_open_connections: config.max_open_connections ?? 10,
     compression: {
       decompress_response: config.compression?.response ?? true,
       compress_request: config.compression?.request ?? false,
@@ -348,10 +348,11 @@ function getConnectionParams<Stream>(
     password: config.password ?? '',
     database: config.database ?? 'default',
     clickhouse_settings: config.clickhouse_settings ?? {},
-    logWriter: new LogWriter(
+    log_writer: new LogWriter(
       config?.log?.LoggerClass
         ? new config.log.LoggerClass()
         : new DefaultLogger(),
+      'Connection',
       config.log?.level
     ),
     additional_headers: config.additional_headers,

packages/client-common/src/connection.ts

@@ -14,7 +14,7 @@ export interface ConnectionParams {
   password: string
   database: string
   clickhouse_settings: ClickHouseSettings
-  logWriter: LogWriter
+  log_writer: LogWriter
   application_id?: string
   additional_headers?: Record<string, string>
 }
@@ -51,6 +51,8 @@ export type ConnPingResult =
     }
   | { success: false; error: Error }
 
+export type ConnOperation = 'Ping' | 'Query' | 'Insert' | 'Exec'
+
 export interface Connection<Stream> {
   ping(): Promise<ConnPingResult>
   close(): Promise<void>

packages/client-common/src/index.ts

@@ -20,6 +20,7 @@ export { ClickHouseError } from './error'
 export {
   ClickHouseLogLevel,
   type ErrorLogParams,
+  type WarnLogParams,
   type Logger,
   type LogParams,
 } from './logger'
@@ -55,7 +56,7 @@ export {
   transformUrl,
   withHttpSettings,
 } from './utils'
-export { LogWriter, DefaultLogger } from './logger'
+export { LogWriter, DefaultLogger, type LogWriterParams } from './logger'
 export { parseError } from './error'
 export type {
   Connection,
@@ -67,6 +68,7 @@ export type {
   ConnBaseResult,
   ConnInsertParams,
   ConnPingResult,
+  ConnOperation,
 } from './connection'
 export {
   type RawDataFormat,