Skip to content
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.

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

Add default HTTP settings + readonly user switch #225

Merged
merged 5 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 34 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
## 0.3.0 (Common, Node.js, Web)

### Breaking changes
slvrtrn marked this conversation as resolved.
Show resolved Hide resolved

slvrtrn marked this conversation as resolved.
Show resolved Hide resolved
- Client will enable [send_progress_in_http_headers](https://clickhouse.com/docs/en/operations/settings/settings#send_progress_in_http_headers) and set `http_headers_progress_interval_ms` to `20000` (20 seconds) by default. These settings in combination allow to avoid LB timeout issues in case of long-running queries without data coming in or out, such as `INSERT FROM SELECT` and similar ones, as the connection could be marked as idle by the LB and closed abruptly. Currently, 20s is chosen as a safe value, since most LBs will have at least 30s of idle timeout, and, for example, AWS LB sends KeepAlive packets every 20s. It can be overridden when creating a client instance if your LB timeout value is even lower than that by manually changing the `send_progress_in_http_headers` value.

NB: these settings will be enabled only if the client instance was created without setting `readonly` flag (see below).

- It is now possible to create a client in read-only mode, which will disable default compression and aforementioned ClickHouse HTTP settings. Previously, if you wanted to use the client with a user created with `READONLY = 1` mode, the response compression had to be disabled explicitly.

Pre 0.3.0:

```ts
const client = createClient({
compression: {
response: false,
},
})
```

With `send_progress_in_http_headers` and `http_headers_progress_interval_ms` settings now enabled by default, this is no longer sufficient. If you need to create a client instance for a read-only user, consider this instead:

```ts
const client = createClient({
readonly: true,
Copy link
Member

Choose a reason for hiding this comment

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

in clickhouse, readonly has a bit different semantics: 0, 1, 2. let's align

})
```

By default, `readonly` is `false`.

NB: this is not necessary if a user has `READONLY = 2` mode as it allows to modify the settings, so the client can be used without an additional `readonly` setting.

See also: [readonly documentation](https://clickhouse.com/docs/en/operations/settings/permissions-for-queries#readonly).

## 0.2.10 (Common, Node.js, Web)

### New features
Expand Down
54 changes: 54 additions & 0 deletions examples/long_running_queries_timeouts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { createClient } from '@clickhouse/client' // or '@clickhouse/client-web'

/**
* If you execute a long-running query without data coming in from the client,
* and your LB has idle connection timeout lower than 300s (it is the default handled by the client under the hood),
* there is a workaround to trigger ClickHouse to send progress HTTP headers
* using `http_headers_progress_interval_ms` setting, which will keep the connection alive from the LB perspective.
*
* One of the symptoms of such LB timeout might be a "socket hang up" error when `request_timeout` runs off,
* but in `system.query_log` the query is marked as completed with its execution time less than `request_timeout`.
*/
void (async () => {
const tableName = 'insert_from_select'
const client = createClient({
/* Here we assume that:

--- we need to execute a long-running query that will not send any data from the client aside from the statement itself,
and will not receive any data from the server during the progress, such as INSERT FROM SELECT,
as there will be an ack only when it's done;
--- there is an LB with 120s idle timeout; a safe value for `http_headers_progress_interval_ms` then will be 110s;
--- the query will take 300-350s to execute; so we choose as safe value of `request_timeout` as 400s.

Of course, the settings values might be different for your particular network configuration */
request_timeout: 400_000,
clickhouse_settings: {
http_headers_progress_interval_ms: '110000', // UInt64, should be passed as a string
},
})
await client.command({
query: `DROP TABLE IF EXISTS ${tableName}`,
})
await client.command({
query: `
CREATE TABLE ${tableName}
(id String, data String)
ENGINE MergeTree()
ORDER BY (id)
`,
})
// Assuming that this is our long-long running insert,
// it should not fail because of LB and the client settings described above.
await client.command({
query: `
INSERT INTO ${tableName}
SELECT '42', 'foobar'
`,
})
const rows = await client.query({
query: `SELECT * FROM ${tableName}`,
format: 'JSONEachRow',
})
console.info(await rows.json())
await client.close()
})()
19 changes: 10 additions & 9 deletions examples/read_only_user.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { createClient } from '@clickhouse/client' // or '@clickhouse/client-web'
import { randomUUID } from 'crypto'

/**
* An illustration of limitations and client-specific settings for users created in `READONLY = 1` mode.
*/
void (async () => {
const defaultClient = createClient()

Expand Down Expand Up @@ -60,9 +63,7 @@ void (async () => {
let readOnlyUserClient = createClient({
username: readOnlyUsername,
password: readOnlyPassword,
compression: {
response: false, // cannot enable HTTP compression for a read-only user
},
readonly: true, // this disables compression and additional ClickHouse settings
})

// read-only user cannot insert the data into the table
Expand Down Expand Up @@ -105,15 +106,15 @@ void (async () => {
console.log('Select result:', await rs.json())
printSeparator()

// ... cannot use compression
// ... cannot use compression or specific ClickHouse HTTP settings
await readOnlyUserClient.close()
readOnlyUserClient = createClient({
username: readOnlyUsername,
password: readOnlyPassword,
compression: {
// this is a default value, but it will cause an error from the ClickHouse side with a read-only user
response: true,
},
/** omitting read-only setting here, and it will cause an error from the ClickHouse side with a read-only user,
* since we enable compression and set `send_progress_in_http_headers` + `http_headers_progress_interval_ms`
* for non-read-only users by default */
// readonly: true,
})

await readOnlyUserClient
Expand All @@ -123,7 +124,7 @@ void (async () => {
})
.catch((err) => {
console.error(
'[Expected error] Cannot use compression with a read-only user. Cause:\n',
`[Expected error] Cannot modify 'send_progress_in_http_headers' setting in readonly mode. Cause:\n`,
err
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ describe('read only user', () => {
insert_quorum: undefined,
database_replicated_enforce_synchronous_settings: undefined,
},
compression: {
response: false, // cannot enable HTTP compression for a read only user
},
readonly: true, // disables compression and additional ClickHouse HTTP settings
})
})
afterAll(async () => {
Expand Down
58 changes: 51 additions & 7 deletions packages/client-common/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {
ClickHouseLogLevel,
ClickHouseSettings,
CompressionSettings,
Connection,
ConnectionParams,
ConnExecResult,
Expand Down Expand Up @@ -49,6 +50,19 @@ export interface ValuesEncoder<Stream> {

export type CloseStream<Stream> = (stream: Stream) => Promise<void>

/**
* By default, {@link send_progress_in_http_headers} is enabled, and {@link http_headers_progress_interval_ms} is set to 20s.
* These settings in combination allow to avoid LB timeout issues in case of long-running queries without data coming in or out,
* such as `INSERT FROM SELECT` and similar ones, as the connection could be marked as idle by the LB and closed abruptly.
* 20s is chosen as a safe value, since most LBs will have at least 30s of idle timeout, and AWS LB sends KeepAlive packets every 20s.
* It can be overridden when creating a client instance if your LB timeout value is even lower than that.
* See also: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html#connection-idle-timeout
*/
const DefaultClickHouseSettings: ClickHouseSettings = {
send_progress_in_http_headers: 1,
http_headers_progress_interval_ms: '20000',
}

export interface ClickHouseClientConfigOptions<Stream> {
impl: {
make_connection: MakeConnection<Stream>
Expand All @@ -65,7 +79,7 @@ export interface ClickHouseClientConfigOptions<Stream> {

compression?: {
/** `response: true` instructs ClickHouse server to respond with
* compressed response body. Default: true. */
* compressed response body. Default: true; if {@link readonly} is enabled, then false. */
response?: boolean
/** `request: true` enabled compression on the client request body.
* Default: false. */
Expand All @@ -81,7 +95,9 @@ export interface ClickHouseClientConfigOptions<Stream> {
application?: string
/** Database name to use. Default value: `default`. */
database?: string
/** ClickHouse settings to apply to all requests. Default value: {} */
/** ClickHouse settings to apply to all requests.
* Default value: {@link DefaultClickHouseSettings}
*/
clickhouse_settings?: ClickHouseSettings
log?: {
/** A class to instantiate a custom logger implementation.
Expand All @@ -90,8 +106,20 @@ export interface ClickHouseClientConfigOptions<Stream> {
/** Default: OFF */
level?: ClickHouseLogLevel
}
/** ClickHouse Session id to attach to the outgoing requests.
* Default: empty. */
session_id?: string
/** Additional HTTP headers to attach to the outgoing requests.
* Default: empty. */
additional_headers?: Record<string, string>
/** If the client instance created for a user with `READONLY = 1` mode,
* some settings, such as {@link compression}, `send_progress_in_http_headers`,
* and `http_headers_progress_interval_ms` can't be modified,
* and will be removed from the client configuration.
* NB: this is not necessary if a user has `READONLY = 2` mode.
* See also: https://clickhouse.com/docs/en/operations/settings/permissions-for-queries#readonly
* Default: false */
readonly?: boolean
}

export type BaseClickHouseClientConfigOptions<Stream> = Omit<
Expand Down Expand Up @@ -335,19 +363,35 @@ function createUrl(host: string): URL {
function getConnectionParams<Stream>(
config: ClickHouseClientConfigOptions<Stream>
): ConnectionParams {
let clickHouseSettings: ClickHouseSettings
let compressionSettings: CompressionSettings
// TODO: maybe validate certain settings that cannot be modified with read-only user
if (!config.readonly) {
clickHouseSettings = {
...DefaultClickHouseSettings,
...config.clickhouse_settings,
}
compressionSettings = {
decompress_response: config.compression?.response ?? true,
compress_request: config.compression?.request ?? false,
}
} else {
clickHouseSettings = config.clickhouse_settings ?? {}
compressionSettings = {
decompress_response: false,
compress_request: false,
}
}
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,
compression: {
decompress_response: config.compression?.response ?? true,
compress_request: config.compression?.request ?? false,
},
compression: compressionSettings,
username: config.username ?? 'default',
password: config.password ?? '',
database: config.database ?? 'default',
clickhouse_settings: config.clickhouse_settings ?? {},
clickhouse_settings: clickHouseSettings,
logWriter: new LogWriter(
config?.log?.LoggerClass
? new config.log.LoggerClass()
Expand Down
10 changes: 6 additions & 4 deletions packages/client-common/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ export interface ConnectionParams {
url: URL
request_timeout: number
max_open_connections: number
compression: {
decompress_response: boolean
compress_request: boolean
}
compression: CompressionSettings
username: string
password: string
database: string
Expand All @@ -19,6 +16,11 @@ export interface ConnectionParams {
additional_headers?: Record<string, string>
}

export interface CompressionSettings {
decompress_response: boolean
compress_request: boolean
}

export interface ConnBaseQueryParams {
query: string
clickhouse_settings?: ClickHouseSettings
Expand Down
1 change: 1 addition & 0 deletions packages/client-common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export {
export { LogWriter, DefaultLogger } from './logger'
export { parseError } from './error'
export type {
CompressionSettings,
Connection,
ConnectionParams,
ConnInsertResult,
Expand Down
2 changes: 1 addition & 1 deletion packages/client-common/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.10'
export default '0.3.0'
57 changes: 52 additions & 5 deletions packages/client-node/__tests__/integration/node_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,58 @@ describe('[Node.js] Client', () => {
await query(client)

expect(httpRequestStub).toHaveBeenCalledTimes(1)
const calledWith = httpRequestStub.calls.mostRecent().args[1]
expect(calledWith.headers).toEqual({
Authorization: 'Basic ZGVmYXVsdDo=', // default user with empty password
const [callURL, callOptions] = httpRequestStub.calls.mostRecent().args
expect(callOptions.headers).toEqual({
'Accept-Encoding': 'gzip',
Authorization: 'Basic ZGVmYXVsdDo=', // default user with empty password
'Test-Header': 'foobar',
'User-Agent': jasmine.stringContaining('clickhouse-js'),
})
assertSearchParams(callURL)
})

it('should work without additional headers', async () => {
const client = createClient({})
await query(client)

expect(httpRequestStub).toHaveBeenCalledTimes(1)
const calledWith = httpRequestStub.calls.mostRecent().args[1]
expect(calledWith.headers).toEqual({
const [callURL, callOptions] = httpRequestStub.calls.mostRecent().args
expect(callOptions.headers).toEqual({
'Accept-Encoding': 'gzip',
Authorization: 'Basic ZGVmYXVsdDo=', // default user with empty password
'User-Agent': jasmine.stringContaining('clickhouse-js'),
})
assertSearchParams(callURL)
})
})

describe('Readonly switch', () => {
it('should disable certain settings by default for a read-only user', async () => {
const client = createClient({ readonly: true })
await query(client)

expect(httpRequestStub).toHaveBeenCalledTimes(1)
const [callURL, callOptions] = httpRequestStub.calls.mostRecent().args
expect(callOptions.headers).toEqual({
// no GZIP header
Authorization: 'Basic ZGVmYXVsdDo=', // default user with empty password
'User-Agent': jasmine.stringContaining('clickhouse-js'),
})
assertReadOnlySearchParams(callURL)
})

it('should behave like default with an explicit false', async () => {
const client = createClient({ readonly: false })
await query(client)

expect(httpRequestStub).toHaveBeenCalledTimes(1)
const [callURL, callOptions] = httpRequestStub.calls.mostRecent().args
expect(callOptions.headers).toEqual({
'Accept-Encoding': 'gzip',
Authorization: 'Basic ZGVmYXVsdDo=', // default user with empty password
'User-Agent': jasmine.stringContaining('clickhouse-js'),
})
assertSearchParams(callURL)
})
})

Expand All @@ -52,4 +84,19 @@ describe('[Node.js] Client', () => {
emitResponseBody(clientRequest, 'hi')
await selectPromise
}

function assertSearchParams(callURL: string | URL) {
const searchParams = new URL(callURL).search.slice(1).split('&')
expect(searchParams).toContain('enable_http_compression=1')
expect(searchParams).toContain('send_progress_in_http_headers=1')
expect(searchParams).toContain('http_headers_progress_interval_ms=20000')
expect(searchParams).toContain(jasmine.stringContaining('query_id='))
expect(searchParams.length).toEqual(4)
}

function assertReadOnlySearchParams(callURL: string | URL) {
const searchParams = new URL(callURL).search.slice(1).split('&')
expect(searchParams).toContain(jasmine.stringContaining('query_id='))
expect(searchParams.length).toEqual(1) // No compression or HTTP settings
}
})
2 changes: 1 addition & 1 deletion packages/client-node/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.10'
export default '0.3.0'
2 changes: 1 addition & 1 deletion packages/client-web/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.10'
export default '0.3.0'
Loading