From fedf157c6c20459d625f593cbe0344081b3082d0 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 14 Aug 2026 10:04:37 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7(yhub)=20allow=20to=20configure=20e?= =?UTF-8?q?very=20createYHub=20parameters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the redis section there were still hard coded values, we want to allow the configurations of this settings. The last part will be the persistence plugin. --- CHANGELOG.md | 11 ++++++++ src/helm/impress/README.md | 2 ++ src/helm/impress/values.yaml | 2 ++ src/yhub-server/README.md | 48 ++++++++++++++++++++++++++++----- src/yhub-server/server.js | 51 ++++++++++++++++++++++++++++-------- 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8993313ed..7d6d09161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,17 @@ and this project adheres to server's. `YHUB_TASK_CONCURRENCY` (default 5, unchanged) sets how many tasks one worker process claims at once — the other half of the throughput knob the replica count is, since redis hands each task to a single worker +- ✨(collaboration) configure the two stream timings that were compiled into + the yhub wrapper: `YHUB_TASK_DEBOUNCE_MS` (default 10000, unchanged), how + long an update waits on the redis stream before a worker persists it — the + delay between an edit and its row in postgres, and the window over which the + edits of a busy document are merged into one task — and + `YHUB_MIN_MESSAGE_LIFETIME_MS` (default 60000, unchanged), how long persisted + updates stay replayable from redis rather than being read back out of + postgres. Neither is a durability setting: the trim never goes past what + postgres holds. Like the concurrency, they are refused at startup when they + are not whole numbers in range, and all of them are logged with the role on + the first line a pod writes - ✨(collaboration) serve two probes on yhub: `GET /collaboration/ping/v1` answers `pong` without touching a store — being answered is what a liveness check should conclude, and restarting a server over a store it cannot reach diff --git a/src/helm/impress/README.md b/src/helm/impress/README.md index ef359fe4d..d59691da7 100644 --- a/src/helm/impress/README.md +++ b/src/helm/impress/README.md @@ -375,6 +375,8 @@ | `yhub.envVars.COLLABORATION_SERVER_ORIGIN` | Comma separated list of the origins allowed to open a websocket | | | `yhub.envVars.YHUB_JWT_PRIVATE_KEY_FILE` | Path to the RSA private key (PEM) yhub signs its calls to the backend with, mounted from a secret | | | `yhub.envVars.YHUB_TASK_CONCURRENCY` | Tasks one worker process claims at once, times the replicas running a worker (default: 5) | | +| `yhub.envVars.YHUB_TASK_DEBOUNCE_MS` | How long an update waits on the redis stream before a worker persists it, in ms (default: 10000) | | +| `yhub.envVars.YHUB_MIN_MESSAGE_LIFETIME_MS` | How long persisted updates stay replayable from redis, in ms (default: 60000) | | | `yhub.envVars.SOFT_MIGRATION` | Set to "true" to seed rooms from the legacy Django/S3 document store on first access | | | `yhub.envVars.BY_VALUE` | Example environment variable by setting value directly | | | `yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.name` | Name of a ConfigMap when configuring env vars from a ConfigMap | | diff --git a/src/helm/impress/values.yaml b/src/helm/impress/values.yaml index 85e90de27..88d646ca5 100644 --- a/src/helm/impress/values.yaml +++ b/src/helm/impress/values.yaml @@ -1014,6 +1014,8 @@ yhub: ## @extra yhub.envVars.COLLABORATION_SERVER_ORIGIN Comma separated list of the origins allowed to open a websocket ## @extra yhub.envVars.YHUB_JWT_PRIVATE_KEY_FILE Path to the RSA private key (PEM) yhub signs its calls to the backend with, mounted from a secret ## @extra yhub.envVars.YHUB_TASK_CONCURRENCY Tasks one worker process claims at once, times the replicas running a worker (default: 5) + ## @extra yhub.envVars.YHUB_TASK_DEBOUNCE_MS How long an update waits on the redis stream before a worker persists it, in ms (default: 10000) + ## @extra yhub.envVars.YHUB_MIN_MESSAGE_LIFETIME_MS How long persisted updates stay replayable from redis, in ms (default: 60000) ## @extra yhub.envVars.SOFT_MIGRATION Set to "true" to seed rooms from the legacy Django/S3 document store on first access ## @extra yhub.envVars.BY_VALUE Example environment variable by setting value directly ## @extra yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.name Name of a ConfigMap when configuring env vars from a ConfigMap diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index a4c282cb6..0a7d97e3f 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -119,13 +119,47 @@ Redis consumer groups hand each task to exactly one worker, so the number of workers is a throughput knob and nothing else: no leader, no partitioning, no coordination between them. -`YHUB_TASK_CONCURRENCY` (default `5`) is the other half of that knob: how many -tasks one process claims at once. What actually runs in parallel is that number -times the processes running a worker, so the two are interchangeable up to the -point where a pod runs out of memory — each task holds the document it merges. -A value that is not a positive integer is refused at startup, like an unknown -role: `Number()` would otherwise read a typo as `NaN` and leave the worker -claiming nothing. +`YHUB_TASK_CONCURRENCY` is the other half of that knob — see below. + +## Tuning + +Three numbers this wrapper passes to yhub, all of them environment variables +whose defaults are what Docs ran with before they were configurable: + +| Variable | Default | What it changes | +| -------- | ------- | --------------- | +| `YHUB_TASK_CONCURRENCY` | `5` | Tasks one worker process claims at once | +| `YHUB_TASK_DEBOUNCE_MS` | `10000` | How long an update waits on the stream before a worker persists it | +| `YHUB_MIN_MESSAGE_LIFETIME_MS` | `60000` | How long persisted updates stay replayable from redis | + +**Concurrency** multiplies with the number of processes running a worker, since +redis hands each task to exactly one of them: the two are interchangeable up to +the point where a pod runs out of memory, each task holding the document it +merges. + +**The debounce** is the delay between an edit and its row in postgres, and the +window over which the edits of a busy document are merged into a single task. +Lowering it persists sooner and compacts more often; raising it does the +reverse. yhub's own default is 120s, which is a long time to lose when a pod is +killed, hence the 10s here. + +**The message lifetime** is not a durability setting: the trim stops at the +older of that age and the point postgres already holds, so nothing unpersisted +is ever dropped. It buys how much recent history a server can replay from redis +instead of reading the document back out of postgres, and it is paid for in +redis memory. + +All three are refused at startup, like an unknown role, when they are not whole +numbers in range (`YHUB_TASK_CONCURRENCY must be an integer >= 1 (got "abc")`): +`Number()` would otherwise read a typo as `NaN` and hand it to yhub, which +takes it — a worker that claims nothing, or a stream that is never trimmed, +with nothing in the logs to say so. Unset and empty both mean the default, so a +kubernetes variable left blank behaves as if it were absent. The effective +values are logged at startup, next to the role: + +```json +{"role":"all","server":true,"worker":true,"taskConcurrency":5,"taskDebounceMs":10000,"minMessageLifetimeMs":60000,"msg":"yhub configuration"} +``` ## Container image diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index 273f9c36a..16697c14d 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -28,10 +28,38 @@ import { migrationLog, } from './migration.js'; +// A numeric setting, read from the environment and refused rather than guessed +// when it is not a whole number at or above `min`: `Number()` reads a typo as +// NaN, which yhub takes as-is and turns into a worker that claims nothing or a +// stream that is never trimmed — a deployment that looks healthy and is not. +// An unset or empty variable is the default, so a kubernetes env var left blank +// behaves as if it had not been set at all. +const intEnv = (name, dflt, min = 1) => { + const raw = process.env[name]; + const value = raw == null || raw === '' ? dflt : Number(raw); + if (!Number.isInteger(value) || value < min) { + throw new Error(`${name} must be an integer >= ${min} (got "${raw}")`); + } + return value; +}; + const PORT = Number(process.env.PORT || 3002); const REDIS = process.env.REDIS; const POSTGRES = process.env.POSTGRES; const REDIS_PREFIX = process.env.REDIS_PREFIX || 'yhub'; +// How long an update waits on the stream before a worker claims the compaction +// task it belongs to. It is the delay between an edit and its row in postgres, +// and the window over which the edits of a busy document are merged into one +// task: lowering it persists sooner and compacts more often, raising it does +// the reverse. yhub defaults to 120s, which is a long time to lose when a pod +// is killed — Docs asks for 10s. +const TASK_DEBOUNCE_MS = intEnv('YHUB_TASK_DEBOUNCE_MS', 10000, 0); +// How long messages a worker has already persisted are kept on the stream. The +// trim stops at the older of that age and the point postgres holds, so this is +// not a durability setting — nothing unpersisted is ever trimmed. It is how +// much recent history stays replayable from redis instead of being read back +// out of postgres, paid for in memory on the redis side. +const MIN_MESSAGE_LIFETIME_MS = intEnv('YHUB_MIN_MESSAGE_LIFETIME_MS', 60000, 0); const COLLABORATION_BACKEND_BASE_URL = process.env.COLLABORATION_BACKEND_BASE_URL || 'http://app-dev:8000'; const allowedOrigins = ( @@ -61,14 +89,8 @@ const RUNS_WORKER = ROLE !== 'server'; // single worker, so what a deployment actually runs in parallel is this times // the number of worker processes — the two knobs are interchangeable up to the // point where a pod runs out of memory, each task holding the document it -// merges. Refused rather than guessed when it is not a positive integer: -// `Number()` would otherwise turn a typo into NaN and yhub into an idle worker. -const TASK_CONCURRENCY = Number(process.env.YHUB_TASK_CONCURRENCY || 5); -if (!Number.isInteger(TASK_CONCURRENCY) || TASK_CONCURRENCY < 1) { - throw new Error( - `YHUB_TASK_CONCURRENCY must be a positive integer (got "${process.env.YHUB_TASK_CONCURRENCY}")`, - ); -} +// merges. +const TASK_CONCURRENCY = intEnv('YHUB_TASK_CONCURRENCY', 5, 1); // Segment every route is mounted under (`server.apiPrefix` below), matching the // URL scheme Docs already routes to the collaboration server. Hardcoded like // the audiences: the backend builds its urls with the same prefix. @@ -843,8 +865,8 @@ const yhub = await createYHub({ redis: { url: REDIS, prefix: REDIS_PREFIX, - taskDebounce: 10000, - minMessageLifetime: 60000, + taskDebounce: TASK_DEBOUNCE_MS, + minMessageLifetime: MIN_MESSAGE_LIFETIME_MS, }, postgres: POSTGRES, persistence: [], // blobs live in yhub's postgres @@ -862,12 +884,19 @@ const yhub = await createYHub({ : null, }); +// What this process was configured to be, in one line: yhub's own startup log +// reports neither the role nor the stream settings, and every one of them is an +// environment variable a deployment can get wrong. The two timings are read +// back off the instance rather than from the constants above, so the line says +// what yhub is using and not merely what it was asked for. logger.info( { role: ROLE, server: RUNS_SERVER, worker: RUNS_WORKER, taskConcurrency: RUNS_WORKER ? TASK_CONCURRENCY : null, + taskDebounceMs: yhub.stream.taskDebounce, + minMessageLifetimeMs: yhub.stream.minMessageLifetime, }, - 'yhub role', + 'yhub configuration', );