mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
chore(clickhouse): upgrade @clickhouse/client from 0.2.10 to 1.23.1 (#4972)
Upgrades @clickhouse/client from 0.2.10 to 1.23.1 in the root package.json and packages/civitai-clickhouse. apps/event-engine was already on 1.x, so the workspace now holds one version of the driver. A version upgrade, not a fix for the ClickHouse socket hang-ups. The pre-upgrade rate was recorded before this change so the post-deploy rate can be compared. - ResultSet.json<T>() returns T[] in 1.x: 13 call sites, 3 in src/ and 10 in apps/moderator, which the root typecheck does not cover. - host -> url; keep_alive.socket_ttl + retry_on_expired_socket -> idle_socket_ttl. - Three 1.x default changes held at their 0.2.x values: max_open_connections Infinity, request_timeout 300000, response compression on. - keep_alive.eagerly_destroy_stale_sockets: true is a deliberate non-default, more permissive than 0.2.x's retry, standing in for it. It confounds the before/after comparison. - 1.x adds ~1ms per request (await sleep(0)); not configurable. - apps/moderator ships on its own release. - New src/server/clickhouse/__tests__/client-config-pins.test.ts pins the values. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Justin Maier
@@ -170,8 +170,8 @@ const startOfUtcDay = (date: Date): Date =>
|
||||
|
||||
/**
|
||||
* How long the charge lookup may take before the caller falls back to the daily mirror. A try/catch
|
||||
* cannot catch a hang and the client sets no request_timeout, so without this its own 30s default
|
||||
* would hold a creator's save open. The query measures ~4ms; this is a fault budget, not a target.
|
||||
* cannot catch a hang, and the shared client's `request_timeout` is 300s, so without this a wedged
|
||||
* read would hold a creator's save open. The query measures ~4ms; this is a fault budget, not a target.
|
||||
*/
|
||||
const CHARGE_LOOKUP_TIMEOUT_MS = 3000;
|
||||
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
// Bound an awaited promise with a timeout that FALLS SOFT instead of hanging.
|
||||
//
|
||||
// A try/catch cannot catch a hang — awaiting a parked promise blocks until the underlying client's own
|
||||
// default fires, which for @clickhouse/client is 30s. Mirrors withTimeoutFallback in the main app
|
||||
// (src/server/utils/timeout-helpers.ts); kept separate because this app shares no server code with it.
|
||||
// `request_timeout` fires, which is 300s for clients built by @civitai/clickhouse. Mirrors
|
||||
// withTimeoutFallback in the main app (src/server/utils/timeout-helpers.ts); kept separate because
|
||||
// this app shares no server code with it.
|
||||
export async function withTimeoutFallback<T>(
|
||||
promise: Promise<T>,
|
||||
ms: number,
|
||||
|
||||
@@ -39,12 +39,13 @@ RUN pnpm --filter @civitai/event-engine build
|
||||
##### prod-deps — pruned, isolated production node_modules ######################
|
||||
# Deploy event-engine's OWN dependency closure (non-legacy). We must NOT use `--legacy`
|
||||
# here: legacy `pnpm deploy` builds a flat/hoisted tree from the WHOLE workspace graph and
|
||||
# hoists a single version of each package to the top level. The repo root (a workspace
|
||||
# member) declares `@clickhouse/client@^0.2.2`; event-engine declares `@clickhouse/client@^1.12.1`
|
||||
# (a major API break). Under `--legacy`, the root's 0.2.x won the hoist, so the built image
|
||||
# shipped `@clickhouse/client@0.2.x` at the top level even though event-engine's own manifest
|
||||
# pins ^1.12.1 — every ClickHouse insert then failed at runtime. Non-legacy deploy resolves
|
||||
# ONLY event-engine's closure and preserves per-package version isolation, so it links 1.12.x.
|
||||
# hoists a single version of each package to the top level. When the repo root declared
|
||||
# `@clickhouse/client@^0.2.2` against event-engine's `^1.12.1` (a major API break), the root's
|
||||
# 0.2.x won the hoist under `--legacy`, so the built image shipped 0.2.x at the top level even
|
||||
# though event-engine's own manifest pinned ^1.12.1 — every ClickHouse insert then failed at
|
||||
# runtime. The root is on ^1.23.1 now, so that particular pair no longer conflicts; the rule
|
||||
# stands because the next diverging dependency would hoist the same way. Non-legacy deploy
|
||||
# resolves ONLY event-engine's closure and preserves per-package version isolation.
|
||||
# pnpm 10 gates non-legacy deploy behind `inject-workspace-packages` — event-engine depends on
|
||||
# NO @civitai/* workspace package, so injection is a no-op here; we just need the gate open.
|
||||
FROM deps AS prod-deps
|
||||
|
||||
@@ -131,8 +131,10 @@ workstreams from `docs/plans/monorepo-migration.md` (in the watcher repo):
|
||||
consumed by BOTH this app and the monolith, then delete the vendored `src/common` here and the monolith's
|
||||
root `event-engine-common` submodule. Note the two EEC copies are at **different commits** today
|
||||
(this app `49b0d4f`; the monolith submodule `7a0c4b0`) — reconcile before sharing.
|
||||
5. **ClickHouse version** — `@clickhouse/client` is `1.12` here vs `0.2.2` at the monorepo root; only needs
|
||||
reconciling if adopting `@civitai/clickhouse`.
|
||||
5. **ClickHouse client** — both this app and the monorepo root are on `@clickhouse/client` 1.x now, so
|
||||
there is no version split left to reconcile. What remains is that this app builds its own client with
|
||||
`createClient` rather than going through `@civitai/clickhouse`, so it takes the library's defaults
|
||||
where the shared client pins them.
|
||||
6. **Meilisearch** — keep this app's own client, or factor a `@civitai/meilisearch` package.
|
||||
7. ~~**DevOps (Zach):** add a Tekton tag-webhook trigger + a `release-app.mjs`/`release:event-engine`
|
||||
entry + the k8s Deployment/HPA/secret (port from the legacy `k8s/09-metric-watcher-app.yml`). The
|
||||
|
||||
@@ -47,7 +47,7 @@ export async function getDownleveledImages({
|
||||
query_params: params,
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const rows = await resp.json<ChRow[]>();
|
||||
const rows = await resp.json<ChRow>();
|
||||
|
||||
let nextCursor: string | undefined;
|
||||
if (limit && rows.length > limit) nextCursor = rows.pop()?.createdAt;
|
||||
|
||||
@@ -140,7 +140,7 @@ export async function removeImagesFromBlocklist(pHashes: (string | null)[]): Pro
|
||||
)}) AND disabled = false`,
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const blocked = await resultSet.json<{ hash: string; reason: string }[]>();
|
||||
const blocked = await resultSet.json<{ hash: string; reason: string }>();
|
||||
if (!blocked.length) return;
|
||||
await ch.insert({
|
||||
table: 'blocked_images',
|
||||
|
||||
@@ -401,7 +401,7 @@ export async function getAppealImageQueue({
|
||||
query_params: { ids },
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
for (const t of await resp.json<{ imageId: number; tosReason: string }[]>())
|
||||
for (const t of await resp.json<{ imageId: number; tosReason: string }>())
|
||||
tosByImage.set(t.imageId, t.tosReason);
|
||||
} catch (e) {
|
||||
console.error('[appeals] tosReason lookup failed', e);
|
||||
|
||||
@@ -66,7 +66,7 @@ export async function getRouteUserBreakdown(
|
||||
query_params: { location, days },
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const rows = await resultSet.json<{ userId: number; visits: number; lastVisit: string }[]>();
|
||||
const rows = await resultSet.json<{ userId: number; visits: number; lastVisit: string }>();
|
||||
if (!rows.length) return [];
|
||||
|
||||
const nameById = await usersByIds(rows.map((r) => r.userId));
|
||||
|
||||
@@ -24,7 +24,7 @@ export async function getTodaysProhibitedPrompts(limit = 500): Promise<Prohibite
|
||||
query_params: { limit },
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
return resultSet.json<ProhibitedPrompt[]>();
|
||||
return resultSet.json<ProhibitedPrompt>();
|
||||
}
|
||||
|
||||
export async function getTodaysProhibitedUserCounts(): Promise<ProhibitedUserCount[]> {
|
||||
@@ -38,6 +38,6 @@ export async function getTodaysProhibitedUserCounts(): Promise<ProhibitedUserCou
|
||||
`,
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const rows = await resultSet.json<{ userId: number; count: string }[]>();
|
||||
const rows = await resultSet.json<{ userId: number; count: string }>();
|
||||
return rows.map((r) => ({ userId: r.userId, count: Number(r.count) }));
|
||||
}
|
||||
|
||||
@@ -158,8 +158,8 @@ export async function listScans(
|
||||
ch.query({ query: countQuery, query_params: params, format: 'JSONEachRow' }),
|
||||
]);
|
||||
|
||||
const rows = await dataResp.json<AggregatedScanRow[]>();
|
||||
const countRows = await countResp.json<Array<{ total: string }>>();
|
||||
const rows = await dataResp.json<AggregatedScanRow>();
|
||||
const countRows = await countResp.json<{ total: string }>();
|
||||
const total = Number(countRows[0]?.total ?? 0);
|
||||
|
||||
if (rows.length === 0) return { rows: [], total };
|
||||
@@ -213,7 +213,7 @@ async function getActiveLabels(scanner: Scanner): Promise<Set<string>> {
|
||||
query_params: { scanner },
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const rows = await resp.json<Array<{ label: string }>>();
|
||||
const rows = await resp.json<{ label: string }>();
|
||||
return new Set(rows.map((r) => r.label));
|
||||
}
|
||||
|
||||
@@ -326,7 +326,7 @@ export async function focusedRun(input: {
|
||||
},
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
const allRows = await resp.json<AggregatedScanRow[]>();
|
||||
const allRows = await resp.json<AggregatedScanRow>();
|
||||
|
||||
const lookbackCutoff = new Date(Date.now() - lookback * 24 * 60 * 60 * 1000);
|
||||
const verdictedInLookbackRow = await dbRead
|
||||
|
||||
+1
-1
@@ -143,7 +143,7 @@
|
||||
"@civitai/orchestration-client": "0.2.0-beta.106",
|
||||
"@civitai/shared": "workspace:*",
|
||||
"@clavata/sdk": "^0.2.3",
|
||||
"@clickhouse/client": "^0.2.2",
|
||||
"@clickhouse/client": "^1.23.1",
|
||||
"@coinbase/cdp-sdk": "^1.13.0",
|
||||
"@discordjs/rest": "^2.6.0",
|
||||
"@dnd-kit/core": "^6.1.0",
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
"main": "./src/index.ts",
|
||||
"types": "./src/index.ts",
|
||||
"dependencies": {
|
||||
"@clickhouse/client": "^0.2.2",
|
||||
"@clickhouse/client": "^1.23.1",
|
||||
"dayjs": "^1.11.12",
|
||||
"zod": "^4.0.17"
|
||||
}
|
||||
|
||||
@@ -52,12 +52,39 @@ export function createClickhouseClient(
|
||||
|
||||
console.log('Creating ClickHouse client');
|
||||
const client = createClient({
|
||||
host: config.host,
|
||||
url: config.host,
|
||||
username: config.username,
|
||||
password: config.password,
|
||||
// Without the retry, a keep-alive socket the server closed while idle is handed to the next
|
||||
// request and fails with ECONNRESET ("socket hang up") instead of reconnecting.
|
||||
keep_alive: { enabled: true, socket_ttl: 2500, retry_on_expired_socket: true },
|
||||
// 2.5s is the client default and stays under the server's 10s keep_alive_timeout, so an idle
|
||||
// socket is retired before the server closes it out from under the next request.
|
||||
//
|
||||
// 🔴 `eagerly_destroy_stale_sockets` is NOT a pin — it is a deliberate non-default, and
|
||||
// 0.2.x had no equivalent. With `retry_on_expired_socket: true`, which this client set, 0.2.x
|
||||
// checked socket age at ASSIGNMENT and retried up to 3 times behind that check; 1.x removed
|
||||
// that option and the retry with it, and stamps the clock at RELEASE, so the age it measures
|
||||
// excludes the query's own duration and is strictly more permissive. This sweep is the closest
|
||||
// 1.x offers, not a restoration: leaving it false would ship less socket protection than
|
||||
// production had, and `false` is no more neutral than `true`. So any change in the socket
|
||||
// hang-up rate after the upgrade is the version and this flag together.
|
||||
keep_alive: { enabled: true, idle_socket_ttl: 2500, eagerly_destroy_stale_sockets: true },
|
||||
// The three values below are pins: 0.2.x resolved each to exactly this and 1.x resolves it to
|
||||
// something else, so setting them keeps the upgrade a transport change and nothing else.
|
||||
//
|
||||
// 1.x defaults to 10. Prod measures ~4 concurrent connections per pod at the busiest second of
|
||||
// the day, so 10 would not bind today — but a request queued behind a full pool gets no timer
|
||||
// at all (`socket.setTimeout` is attached only once a socket is assigned), so a bound turns a
|
||||
// slow request into a hung one.
|
||||
max_open_connections: Infinity,
|
||||
// 1.x defaults to 30_000. 0.2.x resolved 300_000 at runtime, which its own JSDoc contradicted
|
||||
// (`client-common/dist/client.js` read `config.request_timeout ?? 300000`). Held for parity,
|
||||
// not need: some job paths here set no bound of their own, but prod ran no app query past 20s
|
||||
// in the 24h measured before the upgrade. The hot feed read has its own much tighter bound
|
||||
// (CLICKHOUSE_IMAGE_METRICS_TIMEOUT_MS). Above 60_000 and without
|
||||
// `send_progress_in_http_headers`, 1.x warns at construction that a long request_timeout can
|
||||
// itself surface as a socket hang up past a load balancer's idle timeout.
|
||||
request_timeout: 300_000,
|
||||
// 1.x normalizes an unset value to disabled; 0.2.x defaulted it on.
|
||||
compression: { response: true },
|
||||
clickhouse_settings: {
|
||||
async_insert: 1,
|
||||
wait_for_async_insert: 0,
|
||||
|
||||
Generated
+6
-19
@@ -80,8 +80,8 @@ importers:
|
||||
specifier: ^0.2.3
|
||||
version: 0.2.3
|
||||
'@clickhouse/client':
|
||||
specifier: ^0.2.2
|
||||
version: 0.2.10
|
||||
specifier: ^1.23.1
|
||||
version: 1.23.1
|
||||
'@coinbase/cdp-sdk':
|
||||
specifier: ^1.13.0
|
||||
version: 1.33.0(bufferutil@4.0.9)(fastestsmallesttextencoderdecoder@1.0.22)(typescript@5.9.2)(utf-8-validate@5.0.10)
|
||||
@@ -1545,8 +1545,8 @@ importers:
|
||||
packages/civitai-clickhouse:
|
||||
dependencies:
|
||||
'@clickhouse/client':
|
||||
specifier: ^0.2.2
|
||||
version: 0.2.10
|
||||
specifier: ^1.23.1
|
||||
version: 1.23.1
|
||||
dayjs:
|
||||
specifier: ^1.11.12
|
||||
version: 1.11.13
|
||||
@@ -2233,13 +2233,6 @@ packages:
|
||||
resolution: {integrity: sha512-2XTnBkF1MVwicO2sIb9J1mPh34UxZpGka7vtVnnWD1hOMXWIh/eNYZ7XM2k7xix9VKZ9sW4RajeVqPtZaSXx3A==}
|
||||
engines: {node: 22.13.0, npm: 10.9.2}
|
||||
|
||||
'@clickhouse/client-common@0.2.10':
|
||||
resolution: {integrity: sha512-BvTY0IXS96y9RUeNCpKL4HUzHmY80L0lDcGN0lmUD6zjOqYMn78+xyHYJ/AIAX7JQsc+/KwFt2soZutQTKxoGQ==}
|
||||
|
||||
'@clickhouse/client@0.2.10':
|
||||
resolution: {integrity: sha512-ZwBgzjEAFN/ogS0ym5KHVbR7Hx/oYCX01qGp2baEyfN2HM73kf/7Vp3GvMHWRy+zUXISONEtFv7UTViOXnmFrg==}
|
||||
engines: {node: '>=16'}
|
||||
|
||||
'@clickhouse/client@1.23.1':
|
||||
resolution: {integrity: sha512-vs3/Zc1dHvT171btW5nMoPsPCJ6QVJ5pp7obxzO5sjqwFx/jjz9wwCAqcFOdc2DhprugDBaVn+4dVY8hG3A9nw==}
|
||||
engines: {node: '>=20'}
|
||||
@@ -13461,12 +13454,6 @@ snapshots:
|
||||
'@grpc/grpc-js': 1.13.4
|
||||
module-alias: 2.2.3
|
||||
|
||||
'@clickhouse/client-common@0.2.10': {}
|
||||
|
||||
'@clickhouse/client@0.2.10':
|
||||
dependencies:
|
||||
'@clickhouse/client-common': 0.2.10
|
||||
|
||||
'@clickhouse/client@1.23.1': {}
|
||||
|
||||
'@coinbase/cdp-sdk@1.33.0(bufferutil@4.0.9)(fastestsmallesttextencoderdecoder@1.0.22)(typescript@5.9.2)(utf-8-validate@5.0.10)':
|
||||
@@ -17650,9 +17637,9 @@ snapshots:
|
||||
obug: 2.1.1
|
||||
std-env: 4.2.0
|
||||
tinyrainbow: 3.1.0
|
||||
vitest: 4.1.11(@opentelemetry/api@1.9.0)(@types/node@24.13.3)(@vitest/browser-playwright@4.1.11)(@vitest/coverage-v8@4.1.11)(happy-dom@20.9.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(jsdom@27.4.0(@noble/hashes@1.8.0)(bufferutil@4.0.9)(utf-8-validate@5.0.10))(msw@2.12.10(@types/node@24.13.3)(typescript@5.9.2))(vite@6.4.3(@types/node@24.13.3)(jiti@2.7.0)(lightningcss@1.32.0)(sass@1.90.0)(sugarss@5.0.1(postcss@8.5.6))(terser@5.51.2)(tsx@4.20.3)(yaml@2.8.1))
|
||||
vitest: 4.1.11(@opentelemetry/api@1.9.0)(@types/node@20.19.9)(@vitest/browser-playwright@4.1.11)(@vitest/coverage-v8@4.1.11)(happy-dom@20.9.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(jsdom@27.4.0(@noble/hashes@1.8.0)(bufferutil@4.0.9)(utf-8-validate@5.0.10))(msw@2.12.10(@types/node@20.19.9)(typescript@5.9.2))(vite@6.4.3(@types/node@20.19.9)(jiti@2.7.0)(lightningcss@1.32.0)(sass@1.90.0)(sugarss@5.0.1(postcss@8.5.23))(terser@5.51.2)(tsx@4.20.3)(yaml@2.8.1))
|
||||
optionalDependencies:
|
||||
'@vitest/browser': 4.1.11(bufferutil@4.0.9)(msw@2.12.10(@types/node@24.13.3)(typescript@5.9.2))(utf-8-validate@5.0.10)(vite@6.4.3(@types/node@24.13.3)(jiti@2.7.0)(lightningcss@1.32.0)(sass@1.90.0)(sugarss@5.0.1(postcss@8.5.6))(terser@5.51.2)(tsx@4.20.3)(yaml@2.8.1))(vitest@4.1.11)
|
||||
'@vitest/browser': 4.1.11(bufferutil@4.0.9)(msw@2.12.10(@types/node@20.19.9)(typescript@5.9.2))(utf-8-validate@5.0.10)(vite@6.4.3(@types/node@20.19.9)(jiti@2.7.0)(lightningcss@1.32.0)(sass@1.90.0)(sugarss@5.0.1(postcss@8.5.23))(terser@5.51.2)(tsx@4.20.3)(yaml@2.8.1))(vitest@4.1.11)
|
||||
|
||||
'@vitest/expect@4.1.11':
|
||||
dependencies:
|
||||
|
||||
Vendored
+4
-4
@@ -297,9 +297,9 @@ export const serverSchema = z
|
||||
REDIS_CLUSTER_ROUTING_RETRY_BACKOFF_MAX_MS: z.coerce.number().default(150),
|
||||
|
||||
// Upper bound (ms) on a single ClickHouse image-metrics read in the feed/SSR
|
||||
// hot path (getImageMetricsObject). The @clickhouse/client default
|
||||
// request_timeout is 30000ms, so a saturated/cold-cache-miss metric read would
|
||||
// otherwise park ~30s and blow the SSR deadline (the surrounding try/catch
|
||||
// hot path (getImageMetricsObject). The client's own `request_timeout` is
|
||||
// 300000ms, so a saturated/cold-cache-miss metric read would otherwise park for
|
||||
// MINUTES and blow the SSR deadline (the surrounding try/catch
|
||||
// CANNOT catch a hang). The CH metric query (entityMetricDailyAgg_v2) is
|
||||
// genuinely slow — ~4.6s p50 / ~11s p99 — so on a cold cache miss the timeout
|
||||
// fires and we fail SOFT to empty metrics, yielding TRANSIENT zeros. That
|
||||
@@ -309,7 +309,7 @@ export const serverSchema = z
|
||||
// Default 3000ms — snappy SSR over correctness on the first cold render.
|
||||
// .int().positive() so a misconfigured 0 / negative fails fast at BOOT instead
|
||||
// of silently disabling the guard (withTimeoutFallback passes through unbounded
|
||||
// when ms<=0 → the exact ~30s hang this exists to prevent, with no signal).
|
||||
// when ms<=0 → the exact multi-minute hang this exists to prevent, with no signal).
|
||||
CLICKHOUSE_IMAGE_METRICS_TIMEOUT_MS: z.coerce.number().int().positive().default(3000),
|
||||
// Per-read deadline for `/api/user/settings`, which `_app` self-fetches on every SSR
|
||||
// render. Must stay well under `APP_SETTINGS_FETCH_TIMEOUT_MS` (8s): a response the
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import type * as ClickhouseDriver from '@clickhouse/client';
|
||||
|
||||
import { createClickhouseClient } from '@civitai/clickhouse/client';
|
||||
|
||||
const createClient = vi.hoisted(() => vi.fn(() => ({})));
|
||||
vi.mock('@clickhouse/client', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof ClickhouseDriver>()),
|
||||
createClient,
|
||||
}));
|
||||
|
||||
/**
|
||||
* 🔴 IF YOU ARE HERE TO DELETE THESE, READ THIS FIRST.
|
||||
*
|
||||
* `tsc` catches a MISSPELLED top-level or `keep_alive` option, because the config is an
|
||||
* inline literal. It catches nothing else here: every option is optional, so a removed one
|
||||
* compiles, and `clickhouse_settings` accepts any string key, so a misspelled setting
|
||||
* compiles too. The values, and the settings key below, are protected by this file alone.
|
||||
*
|
||||
* `@clickhouse/client` 1.x resolves three of these differently from the 0.2.x the repo
|
||||
* ran until the 1.x upgrade, which deliberately held each at its pre-upgrade value so the
|
||||
* version change moved the transport and nothing else. Dropping one silently adopts the
|
||||
* 1.x default: a bound pool that queues without a socket timer, a request ceiling an
|
||||
* order of magnitude tighter than the jobs on this client need, or responses that cross
|
||||
* the network uncompressed.
|
||||
*
|
||||
* `eagerly_destroy_stale_sockets` is the exception and is NOT a pin: 0.2.x had no
|
||||
* equivalent, and it is a deliberate non-default standing in for the retry 1.x removed.
|
||||
* See the comment beside it in packages/civitai-clickhouse/src/client.ts.
|
||||
*
|
||||
* Change any of them on purpose, with a measurement, and change this test in the same
|
||||
* commit.
|
||||
*/
|
||||
describe('shared ClickHouse client transport config', () => {
|
||||
function buildConfig() {
|
||||
createClient.mockClear();
|
||||
createClickhouseClient({ host: 'http://clickhouse.invalid:8123' });
|
||||
// Not decoration. A factory that stopped calling createClient would otherwise fail as a
|
||||
// TypeError on `calls[0]`, which reads like a broken test file; one that built a second,
|
||||
// unpinned client would otherwise pass silently on the first.
|
||||
expect(createClient).toHaveBeenCalledTimes(1);
|
||||
return createClient.mock.calls[0][0] as Record<string, unknown>;
|
||||
}
|
||||
|
||||
it('pins max_open_connections to Infinity, not the 1.x default of 10', () => {
|
||||
expect(buildConfig().max_open_connections).toBe(Infinity);
|
||||
});
|
||||
|
||||
it('pins request_timeout to 300_000, not the 1.x default of 30_000', () => {
|
||||
expect(buildConfig().request_timeout).toBe(300_000);
|
||||
});
|
||||
|
||||
it('pins response compression on, which 1.x turns off when unset', () => {
|
||||
expect(buildConfig().compression).toEqual({ response: true });
|
||||
});
|
||||
|
||||
// 64-bit integers arrive as strings without this, silently, in every consumer.
|
||||
it('pins the 64-bit integer output format', () => {
|
||||
expect(buildConfig().clickhouse_settings).toMatchObject({
|
||||
output_format_json_quote_64bit_integers: 0,
|
||||
});
|
||||
});
|
||||
|
||||
// The TTL has to stay under the server's 10s keep_alive_timeout; the eager sweep is the
|
||||
// deliberate non-default, asserted here so it cannot be dropped without a decision.
|
||||
it('keeps the idle socket TTL under the server keep-alive and sweeps stale sockets eagerly', () => {
|
||||
expect(buildConfig().keep_alive).toEqual({
|
||||
enabled: true,
|
||||
idle_socket_ttl: 2500,
|
||||
eagerly_destroy_stale_sockets: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -365,7 +365,7 @@ async function imageLeaderboardPopulation(ctx: LeaderboardContext, [min, max]: [
|
||||
format: 'JSONEachRow',
|
||||
});
|
||||
|
||||
const scores = (await response?.json<(ImageScores & { metrics: string })[]>()).map((s) => ({
|
||||
const scores = (await response?.json<ImageScores & { metrics: string }>()).map((s) => ({
|
||||
...s,
|
||||
metrics: JSON.parse(s.metrics) as Record<string, number>,
|
||||
}));
|
||||
|
||||
@@ -248,9 +248,7 @@ async function chCancellableQuery<T extends object>(
|
||||
format: 'JSONEachRow',
|
||||
abort_signal: controller.signal,
|
||||
});
|
||||
// `ResultSet.json<T>()` resolves to `T` itself, not `T[]` — the row type has to
|
||||
// be passed as the array.
|
||||
return await response.json<T[]>();
|
||||
return await response.json<T>();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -181,7 +181,7 @@ export function createBuzzEvent<T>({
|
||||
`,
|
||||
format: 'JSONEachRow',
|
||||
})
|
||||
.then((x) => x.json<{ total: number }[]>())) ?? []
|
||||
.then((x) => x.json<{ total: number }>())) ?? []
|
||||
: [];
|
||||
*/
|
||||
|
||||
|
||||
@@ -2,10 +2,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
// getImageMetricsObject is the metric leg of the getAllImages 12-way Promise.all
|
||||
// fan-out on the image feed / SSR hot path. It reads counts from ClickHouse via
|
||||
// MetricService.fetch, which has NO request-level timeout beyond the
|
||||
// @clickhouse/client 30s default — and a try/catch CANNOT catch a hang. We bound
|
||||
// it with withTimeoutFallback so a wedged read fails SOFT to empty metrics
|
||||
// instead of parking ~30s and blowing the SSR deadline.
|
||||
// MetricService.fetch, which has NO request-level timeout beyond the shared
|
||||
// client's `request_timeout` of 300s — and a try/catch CANNOT catch a hang. We
|
||||
// bound it with withTimeoutFallback so a wedged read fails SOFT to empty metrics
|
||||
// instead of parking for minutes and blowing the SSR deadline.
|
||||
//
|
||||
// We mock the smallest seams: the event-engine-common MetricService class (so
|
||||
// only its `.fetch` is controlled) plus the db/redis/clickhouse clients and env
|
||||
@@ -106,7 +106,7 @@ describe('getImageMetricsObject ClickHouse timeout fail-soft', () => {
|
||||
const result = await getImageMetricsObject([{ id: 1 }, { id: 2 }]);
|
||||
const elapsed = Date.now() - start;
|
||||
|
||||
// The contract: it RESOLVES (does not park ~30s) with the fail-soft shape —
|
||||
// The contract: it RESOLVES (does not park for minutes) with the fail-soft shape —
|
||||
// an empty `{}` metrics map maps to all-null counts per id (callers treat
|
||||
// null fields as "no metrics"). The key assertion is that it returns fast and
|
||||
// never throws.
|
||||
|
||||
@@ -124,7 +124,7 @@ describe('releasePricingSlot', () => {
|
||||
expect(mockMetric).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The lookup sits on a creator's save and the client's own timeout is 30s, so a stalled ClickHouse
|
||||
// The lookup sits on a creator's save and the client's own timeout is 300s, so a stalled ClickHouse
|
||||
// must not hold the save open. Uses fake timers: a real 3s wait would make this the slowest test here.
|
||||
it('gives up on a hung ClickHouse and falls back', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
@@ -228,8 +228,8 @@ describe('getAppViews — latency is bounded, not just errors', () => {
|
||||
|
||||
it('degrades to UNAVAILABLE when the query hangs past the timeout', async () => {
|
||||
// The degrade-don't-throw contract originally covered errors only. A SLOW
|
||||
// ClickHouse is the more likely failure: the driver's own request_timeout
|
||||
// defaults to five minutes, which would hold the whole Promise.all open.
|
||||
// ClickHouse is the more likely failure: the shared client pins request_timeout
|
||||
// to five minutes, which would hold the whole Promise.all open.
|
||||
//
|
||||
// Fake timers so this costs ~0ms instead of a real 10s on every unit run.
|
||||
// The guard is unweakened: the promise still resolves only via the abort
|
||||
|
||||
@@ -188,16 +188,17 @@ describe('getWindowedCollectionRanking', () => {
|
||||
/**
|
||||
* THE RETRY, at the level the transport actually fails.
|
||||
*
|
||||
* `@clickhouse/client` 0.2.10 throws `Socket hang up after 3 retries` the moment every
|
||||
* pooled keep-alive socket it tries is past `keep_alive.socket_ttl` — ~341 times an
|
||||
* hour against the production deployment on 2026-09-11, and once on the single live
|
||||
* `@clickhouse/client` throws `socket hang up` the moment it hands out a pooled
|
||||
* keep-alive socket the server has already closed — ~341 times an hour against the
|
||||
* production deployment on 2026-09-11 (then running 0.2.10, whose message carried an
|
||||
* `after 3 retries` suffix its own retry loop added), and once on the single live
|
||||
* `period=Month` request in that day's ingress access log. Nothing is sent, so
|
||||
* the failure is instantaneous; a second ask opens a fresh connection.
|
||||
*/
|
||||
describe('the retry', () => {
|
||||
it('re-asks once after a Socket-hang-up and serves the ranking', async () => {
|
||||
mockQuery
|
||||
.mockRejectedValueOnce(new Error('ClickHouse query failed: Socket hang up after 3 retries'))
|
||||
.mockRejectedValueOnce(new Error('ClickHouse query failed: socket hang up'))
|
||||
.mockResolvedValueOnce([{ id: 5 }, { id: 8 }]);
|
||||
const result = await getWindowedCollectionRanking({
|
||||
period: MetricTimeframe.Month,
|
||||
@@ -214,17 +215,15 @@ describe('getWindowedCollectionRanking', () => {
|
||||
});
|
||||
|
||||
it('is BOUNDED at CH_RANKING_MAX_ATTEMPTS — it is a retry, not a spin', async () => {
|
||||
mockQuery.mockRejectedValue(
|
||||
new Error('ClickHouse query failed: Socket hang up after 3 retries')
|
||||
);
|
||||
mockQuery.mockRejectedValue(new Error('ClickHouse query failed: socket hang up'));
|
||||
await getWindowedCollectionRanking({ period: MetricTimeframe.Month, now: NOW });
|
||||
expect(mockQuery).toHaveBeenCalledTimes(CH_RANKING_MAX_ATTEMPTS);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE BUDGET IS THE HALF THAT MATTERS, because it is what stops the retry
|
||||
* doubling the failure mode it is NOT for: a 30 s `request_timeout` against a
|
||||
* saturated server. The clock, not an attempt counter, separates the two — so
|
||||
* doubling the failure mode it is NOT for: a slow query running out its
|
||||
* `request_timeout` against a saturated server. The clock, not an attempt counter, separates the two — so
|
||||
* this test makes the first attempt SLOW and asserts there is no second one.
|
||||
*/
|
||||
it('does not retry a failure that already spent the whole budget', async () => {
|
||||
@@ -246,7 +245,7 @@ describe('getWindowedCollectionRanking', () => {
|
||||
mockQuery
|
||||
.mockImplementationOnce(async () => {
|
||||
vi.advanceTimersByTime(CH_RANKING_RETRY_BUDGET_MS - 1);
|
||||
throw new Error('ClickHouse query failed: Socket hang up after 3 retries');
|
||||
throw new Error('ClickHouse query failed: socket hang up');
|
||||
})
|
||||
.mockResolvedValueOnce([{ id: 42 }]);
|
||||
const result = await getWindowedCollectionRanking({
|
||||
|
||||
@@ -131,9 +131,8 @@ function chDateTime(d: Date): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side execution cap, in seconds. The driver's own `request_timeout`
|
||||
* defaults to FIVE MINUTES (@clickhouse/client-common 0.2.10) and
|
||||
* `max_open_connections` to Infinity, neither of which this app overrides — so
|
||||
* Server-side execution cap, in seconds. The shared client is configured with a
|
||||
* `request_timeout` of FIVE MINUTES and an unbounded `max_open_connections` — so
|
||||
* without a bound a merely SLOW ClickHouse (not a down one) holds the whole
|
||||
* `Promise.all` in getMyAppAnalytics open for minutes.
|
||||
*
|
||||
|
||||
@@ -87,25 +87,24 @@ export const CH_RANKING_DEPTH = 10_000;
|
||||
*
|
||||
* 🔴 MORE THAN ONE, BECAUSE THE TRANSPORT UNDER THIS QUERY DROPS REQUESTS AT A RATE A
|
||||
* ONE-SHOT READ CANNOT ABSORB, AND A DROP HERE IS USER-VISIBLE. `@clickhouse/client`
|
||||
* 0.2.10 pools keep-alive sockets and proactively destroys any it hands out that has
|
||||
* been idle longer than `keep_alive.socket_ttl` (2500 ms, set in
|
||||
* `packages/civitai-clickhouse/src/client.ts`). When the socket it is handed is stale
|
||||
* it destroys that socket and asks the agent for another — four times — and then
|
||||
* throws `Socket hang up after 3 retries`, instantly, before a byte reaches
|
||||
* ClickHouse. Measured against the production deployment on 2026-09-11: **~341 such
|
||||
* throws per hour** app-wide, and the single live `period=Month` request in that
|
||||
* day's ingress access log (21:58:27Z) is one of them — it degraded to the all-time Postgres
|
||||
* ordering and rendered the app's "ranking isn't available right now" note.
|
||||
* pools keep-alive sockets and retires any it is about to hand out that has been idle
|
||||
* longer than `keep_alive.idle_socket_ttl` (2500 ms, set in
|
||||
* `packages/civitai-clickhouse/src/client.ts`); when the connection it does hand out
|
||||
* was already closed by the server the request throws `socket hang up` instantly,
|
||||
* before a byte reaches ClickHouse. Measured against the production deployment on
|
||||
* 2026-09-11, then running 0.2.10: **~341 such throws per hour** app-wide, and the
|
||||
* single live `period=Month` request in that day's ingress access log (21:58:27Z) is
|
||||
* one of them — it degraded to the all-time Postgres ordering and rendered the app's
|
||||
* "ranking isn't available right now" note.
|
||||
*
|
||||
* 🔴 THE CLIENT'S OWN FOUR ATTEMPTS ARE NOT A SUBSTITUTE, AND AN ATTEMPT HERE IS BEST
|
||||
* READ AS "DRAIN UP TO FOUR MORE STALE SOCKETS". No `max_open_connections` is
|
||||
* configured, so Node's agent pools without bound and a burst can leave more than four
|
||||
* idle sockets behind; each failed `$query` retires the four it touched, so a further
|
||||
* ask is materially more likely to reach a live or brand-new connection than the one
|
||||
* before it. THREE is therefore a probability improvement, not a guarantee — the
|
||||
* durable fix is at the shared client (bound the pool, or stop handing out sockets
|
||||
* this close to their TTL), which is an app-wide change and deliberately not made from
|
||||
* this feature.
|
||||
* 🔴 AN ATTEMPT HERE IS BEST READ AS "DRAIN ANOTHER STALE SOCKET". The pool is
|
||||
* unbounded (`max_open_connections: Infinity`), so a burst can leave several dead
|
||||
* sockets behind and each failed `$query` retires the one it touched — a further ask
|
||||
* is materially more likely to reach a live or brand-new connection than the one
|
||||
* before it. THREE is therefore a probability improvement, not a guarantee. The shared
|
||||
* client now sweeps stale sockets before handing one out
|
||||
* (`keep_alive.eagerly_destroy_stale_sockets`); whether that removes the need for this
|
||||
* loop is a question for the measured socket-hangup rate, not for this file.
|
||||
*
|
||||
* 🔴 IT IS A RETRY OF A `SELECT`, AND NOTHING ELSE MAY EVER BE RETRIED HERE. This
|
||||
* query reads; it has no side effect to duplicate. A future writer on this path must
|
||||
@@ -120,9 +119,9 @@ export const CH_RANKING_MAX_ATTEMPTS = 3;
|
||||
* is a budget and not a plain attempt count. The failure mode above is instantaneous
|
||||
* (the socket is destroyed locally, nothing is sent), so every attempt it allows fits
|
||||
* inside the budget many times over. The failure mode that must NOT be retried is a
|
||||
* slow one — `@clickhouse/client`'s own 30 s `request_timeout` against a saturated or
|
||||
* unreachable server — because retrying that parks a user-facing discovery request for
|
||||
* a further 30 s to reach the same answer it already had. Elapsed time is the signal
|
||||
* slow one — the client's `request_timeout`, 300 s, against a saturated or unreachable
|
||||
* server — because retrying that parks a user-facing discovery request for a further
|
||||
* 300 s to reach the same answer it already had. Elapsed time is the signal
|
||||
* that separates them, so elapsed time is what is measured.
|
||||
*
|
||||
* 1500 ms sits an order of magnitude above the whole healthy query (Month ranks
|
||||
@@ -208,7 +207,7 @@ export async function getWindowedCollectionRanking({
|
||||
// ATTEMPT LOOP — see CH_RANKING_MAX_ATTEMPTS / CH_RANKING_RETRY_BUDGET_MS. The
|
||||
// budget is measured from the START of the first attempt, not per attempt, so the
|
||||
// whole loop costs at most CH_RANKING_RETRY_BUDGET_MS plus one final attempt —
|
||||
// never CH_RANKING_MAX_ATTEMPTS × the client's own 30 s request_timeout.
|
||||
// never CH_RANKING_MAX_ATTEMPTS × the client's 300 s request_timeout.
|
||||
const startedAt = Date.now();
|
||||
let lastError: unknown;
|
||||
let attempts = 0;
|
||||
|
||||
@@ -5079,10 +5079,10 @@ export const getImageMetricsObject = async (
|
||||
try {
|
||||
const ids = data.map((d) => d.id);
|
||||
|
||||
// The ClickHouse read has NO request-level timeout other than the
|
||||
// @clickhouse/client 30s default, and a try/catch CANNOT catch a hang. Bound
|
||||
// The ClickHouse read has NO request-level timeout other than the client's own
|
||||
// `request_timeout` (300s), and a try/catch CANNOT catch a hang. Bound
|
||||
// it here so a saturated/cold-miss metric read fails SOFT to empty metrics
|
||||
// (callers treat missing ids as null) instead of parking ~30s and blowing the
|
||||
// (callers treat missing ids as null) instead of parking for minutes and blowing the
|
||||
// SSR deadline. Empty `{}` matches the existing catch fallback.
|
||||
const timeoutMs = env.CLICKHOUSE_IMAGE_METRICS_TIMEOUT_MS;
|
||||
// Narrow type flows from this call (`fetch('Image', …)` → Record<number,
|
||||
|
||||
@@ -59,8 +59,8 @@ export async function recordPricingSlot(
|
||||
|
||||
/**
|
||||
* How long the charge lookup may take before the caller falls back to the daily mirror. A try/catch
|
||||
* cannot catch a hang and the client sets no request_timeout, so without this its own 30s default
|
||||
* would hold a creator's save open. The query measures ~4ms; this is a fault budget, not a target.
|
||||
* cannot catch a hang, and the shared client's `request_timeout` is 300s, so without this a wedged
|
||||
* read would hold a creator's save open. The query measures ~4ms; this is a fault budget, not a target.
|
||||
*/
|
||||
const CHARGE_LOOKUP_TIMEOUT_MS = 3000;
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@ import { describe, it, expect, vi } from 'vitest';
|
||||
import { withTimeoutFallback } from '../timeout-helpers';
|
||||
|
||||
// A promise that never settles — simulates a wedged/parked async call (e.g. a
|
||||
// ClickHouse read that hangs until the client's own 30s default).
|
||||
// ClickHouse read that hangs until the client's own 300s request_timeout).
|
||||
const never = () => new Promise<never>(() => {});
|
||||
|
||||
describe('withTimeoutFallback', () => {
|
||||
|
||||
@@ -14,12 +14,13 @@ import type { BlockTokenClaims } from '~/server/middleware/block-scope.middlewar
|
||||
* `getWindowedCollectionRanking` runs; only the ClickHouse CLIENT is doubled, with the
|
||||
* exact error production threw:
|
||||
*
|
||||
* ClickHouse query failed: Socket hang up after 3 retries
|
||||
* ClickHouse query failed: socket hang up
|
||||
*
|
||||
* `@clickhouse/client` 0.2.10 raises that when every pooled keep-alive socket it tries
|
||||
* has been idle past `keep_alive.socket_ttl` — measured at ~341 throws per hour against
|
||||
* the production deployment on 2026-09-11, and the single live `period=Month` request
|
||||
* in that day's ingress access log (21:58:27Z) is one of them.
|
||||
* `@clickhouse/client` raises that when it hands out a pooled keep-alive socket the
|
||||
* server has already closed — measured at ~341 throws per hour against the production
|
||||
* deployment on 2026-09-11 (then running 0.2.10, whose own retry loop added an
|
||||
* `after 3 retries` suffix that 1.x no longer emits), and the single live `period=Month`
|
||||
* request in that day's ingress access log (21:58:27Z) is one of them.
|
||||
*
|
||||
* 🔴 AND THE REQUEST IS THE LITERAL PRODUCTION QUERY STRING, parsed the way the server
|
||||
* parses it, rather than a hand-built object. The whole defect class here is "the state
|
||||
@@ -30,8 +31,8 @@ import type { BlockTokenClaims } from '~/server/middleware/block-scope.middlewar
|
||||
|
||||
const PRODUCTION_QUERY_STRING = 'mode=public&sort=Most+Followers&period=Month&limit=24';
|
||||
|
||||
/** The verbatim message `@clickhouse/client` 0.2.10 throws on an exhausted socket pool. */
|
||||
const SOCKET_HANG_UP = 'ClickHouse query failed: Socket hang up after 3 retries';
|
||||
/** The verbatim message `@clickhouse/client` throws on a dead pooled socket. */
|
||||
const SOCKET_HANG_UP = 'ClickHouse query failed: socket hang up';
|
||||
|
||||
function createMocks(query: Record<string, unknown>) {
|
||||
const req = {
|
||||
|
||||
Reference in New Issue
Block a user