comet-ml
diff --git a/‎apps/opik-backend/data-migrations/traces-local-v2-cutover/README.md‎
Lines changed: 107 additions & 11 deletions b/‎apps/opik-backend/data-migrations/traces-local-v2-cutover/README.md‎
Lines changed: 107 additions & 11 deletions
@@ -73,14 +73,17 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
 6. **Cutover buffer knob ready** — `databaseAnalytics.asyncInsertBusyTimeoutMaxMs` (env
    `ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS`), unset by default so the buffer inherits the
    `async_insert_busy_timeout_max_ms=250` carried by `queryParameters`. Raise it to ~10000 for the cutover, then unset it
-   again. The ceiling is a backend per-query setting applied on the backend's own ClickHouse client, so the migration
-   scripts' direct `clickhouse-client` session **cannot read or verify it**. It is therefore **operator-asserted**:
-   `exchange_and_wrap.sh` refuses the EXCHANGE without `--confirm-buffer-raised` (a fail-fast acknowledgment gate — it
-   forces the operator to confirm the step, though it cannot prove the value took effect). Confirm it actually took
-   effect on the prod-clone/staging load test (the Go/No-Go "Async-insert ceiling confirmed" item) before production.
-   **Also confirm client/SDK insert timeouts
-   exceed the widened buffer** (~10s) — with `wait_for_async_insert=1` a raised ceiling blocks each insert until it
-   flushes, so a shorter client timeout would surface as ingestion errors during the window.
+   again. **Where it is set, the exact value, the rollout and the revert step are in
+   ["Where the buffer bump lives"](#where-the-buffer-bump-lives-and-how-to-revert-it)** — it is a temporary env var on the
+   environment overlay, not a chart value (OPIK-7686). Have the overlay change written and reviewed *before* the window,
+   so applying it is a merge, not an edit. The ceiling is a backend per-query setting applied on the backend's own
+   ClickHouse client, so the migration scripts' direct `clickhouse-client` session **cannot read or verify it**. It is
+   therefore **operator-asserted**: `exchange_and_wrap.sh` refuses the EXCHANGE without `--confirm-buffer-raised` (a
+   fail-fast acknowledgment gate — it forces the operator to confirm the step, though it cannot prove the value took
+   effect). Confirm it actually took effect on the prod-clone/staging load test (the Go/No-Go "Async-insert ceiling
+   confirmed" item) before production. **Also confirm client/SDK insert timeouts exceed the widened buffer** (~10s) —
+   with `wait_for_async_insert=1` a raised ceiling blocks each insert until it flushes, so a shorter client timeout would
+   surface as ingestion errors during the window.
 7. **Schema-state flag wired, with a rollout plan** — `databaseAnalyticsDataModel.traceColumnsNonNullable` (env
    `ANALYTICS_DB_DATA_MODEL_TRACE_COLUMNS_NON_NULLABLE`, default `false`). The successor's `end_time`/`ttft` are
    **non-nullable sentinel** columns, so the app must represent an absent value as the epoch/NaN sentinel — not `null` —
@@ -158,7 +161,9 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
    It executes the reference statement in
    [`000001_backfill_traces_local_v2.sql`](scripts/db-app-analytics/000001_backfill_traces_local_v2.sql) — the script
    reads that file and substitutes the window bounds, so the two never drift.
-2. **Raise the buffer ceiling** (config, see below), then **[`scripts/delta_replay.sh`](scripts/delta_replay.sh)**
+2. **Raise the buffer ceiling** (config — see
+   ["Where the buffer bump lives"](#where-the-buffer-bump-lives-and-how-to-revert-it) for the overlay entry, the value and
+   the rollout wait), then **[`scripts/delta_replay.sh`](scripts/delta_replay.sh)**
    (reference SQL [`000002_delta_and_deletion_replay.sql`](scripts/db-app-analytics/000002_delta_and_deletion_replay.sql))
    — delta-insert (anchored at `backfill_start`), then **deletion replay**. The replay runs with
    `lightweight_deletes_sync = 2`, so it returns only once the delete mutation has applied on **every** replica.
@@ -263,6 +268,88 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
 delta one). This is normal — `ReplacingMergeTree` collapses them on merge / under `FINAL` / `LIMIT 1 BY id`, highest
 `last_updated_at` winning. Do not "fix" it.
 
+### Where the buffer bump lives (and how to revert it)
+
+**Decision (OPIK-7686): a temporary env var on the deployed chart's environment overlay — not a chart value.** The three
+`ANALYTICS_DB_ASYNC_INSERT_*` knobs are deliberately absent from the Opik chart's `values.yaml` (OPIK-6880, #7675).
+Rationale:
+
+- **No chart change is needed.** `component.backend.env` is a free-form map rendered straight into the backend
+  ConfigMap, so an overlay entry is already sufficient.
+- **Removal is a clean one-step rollback.** Unset means "leave `queryParameters` alone" (`DatabaseAnalyticsFactory`), so
+  *deleting* the key restores the baked-in `async_insert_busy_timeout_max_ms=250`. There is no "set it back to 250" edit,
+  and therefore no pinned value that can later drift from the `config.yml` default. For a time-boxed window that
+  reversibility is the property worth optimising for.
+- **The value is environment- and window-specific** — prod, during the cutover. An overlay keeps it version-controlled
+  and auditable without turning a temporary state into a permanent chart default that every self-hosted install
+  inherits.
+- **A chart value would save no work**: you edit the overlay either way.
+
+> For the record, the reason first given on #7675 for excluding these — that rendering them would send empty strings
+> where the backend expects an integer, so it "would not be inert" — was **wrong**. `config.yml` ships
+> `${ANALYTICS_DB_ASYNC_INSERT_*:-}` as the default for all three, so the empty case is the normal path in every
+> environment today: an empty substitution leaves a bare YAML scalar that parses to a null `Integer`, and `@Min(1)` does
+> not fire on null. Exposing them in the chart with empty defaults *would* be safe. The decision above rests on
+> reversibility and scope, not on safety.
+
+> **Never bump it by editing `ANALYTICS_DB_QUERY_PARAMETERS`.** That means re-pasting the entire tuning string
+> (`compress`, `failover`, `async_insert`, `wait_for_async_insert`, the skip-index and shard settings, …), which risks
+> silently dropping one of the others and drifting from the `config.yml` default. The dedicated
+> `ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS` override exists precisely so the cutover states only the one value it
+> is changing.
+
+**Comet SaaS (the production cutover).** The overlay is `values-production-opik.yaml` in `comet-ml/comet-helm`, under
+`opik.component.backend.env` (the `opik` chart is a subchart there, so the values are namespaced under `opik:`):
+
+```yaml
+opik:
+  component:
+    backend:
+      env:
+        # OPIK-6875 traces cutover ONLY — delete this line to revert (do not set 250).
+        ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS: "10000"
+```
+
+Only the **ceiling** changes. Leave `…_MIN_MS` and `…_MAX_DATA_SIZE` unset: the floor stays at the
+`async_insert_busy_timeout_min_ms=100` carried by `queryParameters`, and widening the ceiling alone is what parks the
+inserts.
+
+**The rollout is a pod restart, and it happens by itself.** The backend reads the ConfigMap through
+`envFrom.configMapRef: opik-backend`, which Kubernetes injects at container start only — editing the ConfigMap does not
+reach a running pod. The chart annotates the pods `reloader.stakater.com/auto: "true"` and Stakater Reloader runs on the
+cluster, so merging the overlay change triggers the rolling restart on its own; there is no `kubectl rollout restart` to
+issue. What the operator **must** do is wait for that roll to complete before continuing, because the ceiling has to be
+live on *every* instance:
+
+```bash
+kubectl rollout status deployment/opik-backend -n cometml-production
+kubectl get cm opik-backend -n cometml-production \
+    -o jsonpath='{.data.ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS}{"\n"}'
+# and confirm no surviving pod predates the roll:
+kubectl get pods -n cometml-production -l component=opik-backend \
+    -o custom-columns=NAME:.metadata.name,START:.status.startTime
+```
+
+Four consequences to plan for:
+
+- **One roll, not two, before the tail.** `traceColumnsNonNullable = true` (prereq 7) lives in the same overlay, the
+  same ConfigMap and the same Reloader roll — and step 1 of "The final cutover window" asks for both. Land them in
+  **one commit** so the fleet restarts once.
+- **Keep the chosen ceiling below the pod's termination grace period.** The revert is delivered by a second rolling
+  restart, and at that moment pods are holding inserts parked for up to the widened ceiling. Neither the chart nor the
+  overlay sets `terminationGracePeriodSeconds`, so it is the Kubernetes default **30s** — comfortably above the ~10000ms
+  ceiling, but a much larger ceiling would let `SIGKILL` land on parked inserts. Check the two numbers against each other
+  before picking a value.
+- **The roll itself costs ingestion capacity** (rolling-update `maxUnavailable` plus the backend PDB), so do it while
+  there is slack — not between the final delta and the EXCHANGE.
+- **Reloader restarts the fleet on *any* change to that ConfigMap.** Hold unrelated overlay edits for the whole window,
+  the same way prereq 12 freezes DDL.
+
+**Self-hosted / on-premise.** Same env var and the same "delete to revert" semantics; only the delivery differs — a
+`component.backend.env` entry in whatever values file the install uses, or `ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS`
+in the compose `.env` (`docker-compose.yaml` already forwards all three). General tuning guidance for these knobs lives
+in the self-host [ClickHouse async-insert notes](../../../opik-documentation/documentation/fern/docs/self-host/troubleshooting.mdx).
+
 ### The final cutover window (the zero-loss invariant)
 
 The buffer widening (prereq 6) is what makes the flip lossless, but the guarantee rests on a timing invariant worth
@@ -273,7 +360,9 @@ still land in the *old* `traces` in the gap between the last delta read and the
 run. The binding constraint is therefore **not** "replay < buffer window"; it is that the **gap between the final delta
 and the `EXCHANGE` completing must stay within the buffer hold**. So run the tail as tightly as possible:
 
-1. Widen the buffer, and **roll out `traceColumnsNonNullable = true` to every backend instance** (see below).
+1. Widen the buffer, and **roll out `traceColumnsNonNullable = true` to every backend instance** (see below). Both are
+   entries in the same overlay and the same backend ConfigMap, so land them in **one commit** and let the single restart
+   carry both — see ["Where the buffer bump lives"](#where-the-buffer-bump-lives-and-how-to-revert-it).
 2. Do the QA verify on an **earlier** pass (it can take minutes on a large table — do not let it be the last thing
    before the swap).
 3. Run a **final** `delta_replay.sh` as the last write-facing step.
@@ -728,7 +817,8 @@ server's major version, either way:
   `CLICKHOUSE_HOST`; for a ClickHouse on the host's own loopback, add `--network=host` via `CLICKHOUSE_CLIENT_DOCKER_OPTS`.
 
 **The only manual actions are not SQL:** (1) raising/restoring the async-insert buffer ceiling
-(`databaseAnalytics.asyncInsertBusyTimeoutMaxMs`) around steps 2–3; (2) flipping
+(`databaseAnalytics.asyncInsertBusyTimeoutMaxMs`) around steps 2–3 — see
+["Where the buffer bump lives"](#where-the-buffer-bump-lives-and-how-to-revert-it); (2) flipping
 `databaseAnalyticsDataModel.traceColumnsNonNullable` to `true` in lockstep with the EXCHANGE (and back on rollback) —
 see "The final cutover window"; and (3) the go/no-go judgement between steps. All three are *backend config* / judgement changes (env + rolling
 restart, or a config push) that these DB-facing scripts cannot and should not make. They are deliberately operator-owned;
@@ -1079,6 +1169,12 @@ cheap (stage A); the bridge stays enabled so nothing is lost on a retry.
 - [ ] **Async-insert ceiling confirmed** — raising `asyncInsertBusyTimeoutMaxMs` demonstrably widens the adaptive buffer
       under load, not just the cap. `exchange_and_wrap.sh` enforces the acknowledgment via `--confirm-buffer-raised`, but
       that is an assertion only — this checklist item is the actual "it took effect under load" verification.
+- [ ] **The buffer-bump and revert changes are pre-written and reviewed** — the overlay entry raising
+      `ANALYTICS_DB_ASYNC_INSERT_BUSY_TIMEOUT_MAX_MS` (with `traceColumnsNonNullable = true` in the same commit) and the
+      revert that *deletes* the key, both prepared before the window so each is a merge rather than an edit under
+      pressure. Confirm the chosen ceiling is below `terminationGracePeriodSeconds` and that the Reloader-driven roll is
+      expected to complete inside the schedule — see
+      ["Where the buffer bump lives"](#where-the-buffer-bump-lives-and-how-to-revert-it).
 - [ ] **Data Retention confirmed disabled** for the cutover window (`RETENTION_ENABLED=false`). Retention deletes bypass
       the deletion bridge, so a sweep in the window would leak/resurrect across the swap; `exchange_and_wrap.sh` and
       `rollback.sh` (stages B/C) enforce `--confirm-retention-paused`, but that is an assertion — this item is the real