Skip to content

Commit 80df6c0

Browse files
andrescrzclaude
andcommitted
fix(cutover): gate EXCHANGE/rollback on retention-paused; make delete_traffic best-effort
Address Baz round: - exchange_and_wrap.sh / rollback.sh: require --confirm-retention-paused on every EXCHANGE path and on rollback stages B/C. Retention deletes bypass the deletion bridge, so a sweep in the cutover/rollback window leaks live across the swap (forward) or resurrects a deleted row from the backup (reverse-replay). It is an operator assertion — a backend setting the script can't read — so the README Go/No-Go keeps the real "actually paused on every backend" verification. - delete_traffic.py (#851): larger newest-page refill and more generous empty-refill tolerance so mask-visibility lag doesn't end the run early; docstring states it is a best-effort traffic generator, not a guaranteed full-drain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7dbda23 commit 80df6c0

4 files changed

Lines changed: 53 additions & 9 deletions

File tree

apps/opik-backend/data-migrations/traces-local-v2-cutover/README.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,13 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
144144
leaving `traces` a `MergeTree` where deletes still work); the `RENAME` + `Distributed` wrap runs only with
145145
`--with-wrap`. Restore the buffer ceiling and verify.
146146
```bash
147-
CLICKHOUSE_HOST=<host> CLICKHOUSE_PASSWORD=<pw> ./scripts/exchange_and_wrap.sh --database opik --confirm-buffer-raised
147+
CLICKHOUSE_HOST=<host> CLICKHOUSE_PASSWORD=<pw> ./scripts/exchange_and_wrap.sh --database opik \
148+
--backfill-start '<anchor from backfill.sh>' --confirm-buffer-raised --confirm-retention-paused
148149
```
149-
`--confirm-buffer-raised` is required on every EXCHANGE path: it asserts the async-insert buffer is raised so writes in
150-
the final window survive the swap (add `--with-wrap --confirm-daos-retargeted` only once the DAOs target `traces_local`).
150+
Every EXCHANGE path requires: `--backfill-start` (for the final deletion replay), `--confirm-buffer-raised` (writes in
151+
the final window survive the swap), and `--confirm-retention-paused` (retention deletes bypass the bridge, so a
152+
retention sweep in the window would leak across the swap). Add `--with-wrap --confirm-daos-retargeted` only once the
153+
DAOs target `traces_local`.
151154

152155
> **HARD PREREQUISITE for the wrap (step 4, part 2): the delete/mutation DAO must target `traces_local` first.** A
153156
> `Distributed` table supports `SELECT` and `INSERT` but **not** mutations. Verified on ClickHouse 26.3:
@@ -613,7 +616,10 @@ cheap (stage A); the bridge stays enabled so nothing is lost on a retry.
613616
- [ ] **Async-insert ceiling confirmed** — raising `asyncInsertBusyTimeoutMaxMs` demonstrably widens the adaptive buffer
614617
under load, not just the cap. `exchange_and_wrap.sh` enforces the acknowledgment via `--confirm-buffer-raised`, but
615618
that is an assertion only — this checklist item is the actual "it took effect under load" verification.
616-
- [ ] **Data Retention confirmed disabled** for the cutover window (`RETENTION_ENABLED=false`).
619+
- [ ] **Data Retention confirmed disabled** for the cutover window (`RETENTION_ENABLED=false`). Retention deletes bypass
620+
the deletion bridge, so a sweep in the window would leak/resurrect across the swap; `exchange_and_wrap.sh` and
621+
`rollback.sh` (stages B/C) enforce `--confirm-retention-paused`, but that is an assertion — this item is the real
622+
"it is actually paused on every backend" verification.
617623
- [ ] **Reconciliation clean** — per-window source/dest counts within 0.01% across the whole backfill.
618624
- [ ] **Replication settled before the EXCHANGE**`replication_queue` empty and the deletion-replay mutation
619625
`is_done` on **all** replicas (`exchange_and_wrap.sh` gates on this; do not `--force` past it in production).

apps/opik-backend/data-migrations/traces-local-v2-cutover/scripts/exchange_and_wrap.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@
4747
# writes across the swap so they land on the new table; at the default, a write in the final window
4848
# can commit to the old table and be lost after the EXCHANGE. It's a backend setting the script can't
4949
# read, so the operator must assert it.
50+
# --confirm-retention-paused REQUIRED for every EXCHANGE path. Retention deletes (deleteForRetention*) bypass the
51+
# deletion bridge and are never replayed onto the successor, so a retention sweep during the cutover
52+
# window leaks live across the swap. Asserts retention is paused (RETENTION_ENABLED=false on every
53+
# backend) for the whole window — a backend setting the script can't read.
5054

5155
set -euo pipefail
5256

@@ -63,6 +67,7 @@ FORCE=0
6367
CONFIRM_MAINTENANCE=0
6468
CONFIRM_DAOS_RETARGETED=0
6569
CONFIRM_BUFFER_RAISED=0
70+
CONFIRM_RETENTION_PAUSED=0
6671

6772
while [[ $# -gt 0 ]]; do
6873
case "$1" in
@@ -75,6 +80,7 @@ while [[ $# -gt 0 ]]; do
7580
--confirm-maintenance) CONFIRM_MAINTENANCE=1; shift ;;
7681
--confirm-daos-retargeted) CONFIRM_DAOS_RETARGETED=1; shift ;;
7782
--confirm-buffer-raised) CONFIRM_BUFFER_RAISED=1; shift ;;
83+
--confirm-retention-paused) CONFIRM_RETENTION_PAUSED=1; shift ;;
7884
*) echo "Unknown argument: $1" >&2; exit 2 ;;
7985
esac
8086
done
@@ -124,6 +130,16 @@ if [[ "$WRAP_ONLY" != "1" && -z "$BACKFILL_START" ]]; then
124130
echo "ERROR: the EXCHANGE requires --backfill-start (the anchor printed by backfill.sh) for the final deletion replay." >&2
125131
exit 2
126132
fi
133+
# Retention deletes (TraceDAO.deleteForRetention*) bypass the deletion bridge, so they are never replayed onto the
134+
# successor — if any backend still has RETENTION_ENABLED=true, a retention sweep in the cutover window leaks live across
135+
# the swap. Retention is a backend setting the script can't read, so require the operator to assert it is paused for the
136+
# whole window. Applies to every EXCHANGE path (not --wrap-only, which does no data cutover).
137+
if [[ "$WRAP_ONLY" != "1" && "$CONFIRM_RETENTION_PAUSED" != "1" ]]; then
138+
echo "ERROR: the EXCHANGE requires --confirm-retention-paused. Retention deletes bypass the deletion bridge, so a" >&2
139+
echo " retention sweep during the cutover window would leak live across the swap. Pause retention" >&2
140+
echo " (RETENTION_ENABLED=false on every backend) for the whole window, then re-run with the flag." >&2
141+
exit 2
142+
fi
127143

128144
ch() {
129145
clickhouse-client --database "$DATABASE" --log_comment 'traces_local_v2_cutover:exchange_and_wrap' --query "$1"

apps/opik-backend/data-migrations/traces-local-v2-cutover/scripts/rollback.sh

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
# --stage A backfill/delta ran but the EXCHANGE did not — discard the shadow (live `traces` is untouched).
88
# --stage B the EXCHANGE ran but not the wrap — swap the tables back, then reverse-replay.
99
# --stage C the wrap ran — drop the wrapper, promote the parked original, then reverse-replay.
10-
# Stages B and C need --cutover-start (printed by exchange_and_wrap.sh) to bound the reverse-replay. Keep the deletion
11-
# bridge enabled through the rollback so no delete is lost.
10+
# Stages B and C need --cutover-start (printed by exchange_and_wrap.sh) to bound the reverse-replay, and
11+
# --confirm-retention-paused (retention deletes bypass the bridge, so a retention sweep in the rollback window would
12+
# resurrect a deleted row from the backup). Keep the deletion bridge enabled through the rollback so no delete is lost.
1213
#
1314
# SAFETY: the stages are mutually exclusive and each lives in its OWN file, so no single file mixes a TRUNCATE with an
1415
# EXCHANGE/DROP — running any file does exactly one stage. Before running, this asserts the live `traces` topology matches
@@ -26,12 +27,14 @@ SQL_DIR="$SCRIPT_DIR/db-app-analytics"
2627
DATABASE=""
2728
STAGE=""
2829
CUTOVER_START=""
30+
CONFIRM_RETENTION_PAUSED=0
2931

3032
while [[ $# -gt 0 ]]; do
3133
case "$1" in
3234
--database) DATABASE="${2:?"$1 requires a value"}"; shift 2 ;;
3335
--stage) STAGE="${2:?"$1 requires a value"}"; shift 2 ;;
3436
--cutover-start) CUTOVER_START="${2:?"$1 requires a value"}"; shift 2 ;;
37+
--confirm-retention-paused) CONFIRM_RETENTION_PAUSED=1; shift ;;
3538
*) echo "Unknown argument: $1" >&2; exit 2 ;;
3639
esac
3740
done
@@ -44,6 +47,16 @@ case "$STAGE" in
4447
A|B|C) ;;
4548
*) echo "ERROR: --stage must be A, B or C" >&2; exit 2 ;;
4649
esac
50+
# Stages B/C run the reverse-replay, which — like the forward replay — only re-applies bridged deletes. Retention deletes
51+
# (deleteForRetention*) bypass the bridge, so a retention sweep during the rollback window would restore a legitimately
52+
# deleted row from the backup and resurrect it. Retention is a backend setting the script can't read; require the
53+
# operator to assert it is paused. Stage A does no reverse-replay, so it is exempt.
54+
if [[ ( "$STAGE" == "B" || "$STAGE" == "C" ) && "$CONFIRM_RETENTION_PAUSED" != "1" ]]; then
55+
echo "ERROR: rollback --stage $STAGE requires --confirm-retention-paused. The reverse-replay only re-applies bridged" >&2
56+
echo " deletes; a retention sweep in the rollback window would resurrect a deleted row from the backup." >&2
57+
echo " Pause retention (RETENTION_ENABLED=false on every backend), then re-run with the flag." >&2
58+
exit 2
59+
fi
4760

4861
ch() {
4962
clickhouse-client --database "$DATABASE" --log_comment 'traces_local_v2_rollback' --query "$1"

tests_load/tests/traces-local-v2-cutover/delete_traffic.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
It pulls a pool of existing trace ids via search and deletes them at the target rate, refilling as it drains. Run it
88
during or after the backfill so the traces it deletes have already been copied — that is the leak the bridge prevents.
99
10+
This is a best-effort TRAFFIC GENERATOR (deletes newest-first for the run's --duration), NOT a guaranteed full-drain:
11+
search returns the newest page, so during delete-mask visibility lag a refill can transiently return only ids already
12+
in `seen`/`pool`; the loop tolerates a few such empty refills (EMPTY_REFILL_LIMIT) before concluding the project is
13+
drained. It does not assert every trace was deleted — its job is to exercise the deletion bridge, not to empty the table.
14+
1015
Prerequisites: `OPIK_URL_OVERRIDE` pointing at the local install. Run `python delete_traffic.py --help` for options.
1116
"""
1217

@@ -20,8 +25,12 @@
2025
_stop = False
2126

2227
# Consecutive empty refills tolerated before concluding the project is drained. An empty refill can be a transient
23-
# delete-mask-visibility lag (the just-deleted top ids not yet hidden from search), not a truly empty project.
24-
EMPTY_REFILL_LIMIT = 3
28+
# delete-mask-visibility lag (the just-deleted top ids not yet hidden from search), not a truly empty project — so give
29+
# the mask several beats to propagate before stopping.
30+
EMPTY_REFILL_LIMIT = 5
31+
# Newest-page size to pull per refill. Larger reaches past the just-deleted (still-visible) top ids to undeleted ones
32+
# during mask lag, so the run keeps finding work instead of stopping early.
33+
REFILL_FETCH = 2000
2534

2635

2736
def _handle_sigint(_signum, _frame):
@@ -61,7 +70,7 @@ def main(project, tps, duration, batch):
6170
tick = time.time()
6271
if len(pool) < batch:
6372
# Exclude both already-deleted ids and those still queued in `pool`, so a refill can't requeue an in-flight id.
64-
fetched = _fetch_ids(client, project, want=500, exclude=seen | set(pool))
73+
fetched = _fetch_ids(client, project, want=REFILL_FETCH, exclude=seen | set(pool))
6574
if fetched is None:
6675
# transient search failure — back off and retry rather than mistaking it for "no more traces".
6776
time.sleep(interval if interval > 0 else 0.5)

0 commit comments

Comments
 (0)