Skip to content

Commit 1b7ba3b

Browse files
committed
test(cutover): address second Baz review — wrap-only split-state guard + fixes
- exchange_and_wrap.sh --wrap-only now refuses when traces_local_v2 still exists (EXCHANGE done but post-swap RENAME not): wrapping then would orphan the old data under the wrong name. Prints the rename to finish first. - seed_history: build BAD_ID_INSTANT from a fixed date so it can't raise ValueError at import on a Feb-29 run (2201 is not a leap year). - delete_traffic: a transient search_traces failure now backs off and retries instead of being mistaken for "no more traces" and ending the run. - README: document the mutually-exclusive wrap flags and EXCHANGE-only default; reword the awkward "EXCHANGEd" to "already-swapped".
1 parent 0560981 commit 1b7ba3b

4 files changed

Lines changed: 28 additions & 9 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,12 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
155155
>
156156
> **Applying the deferred wrap later:** once the sharding-aware DAO has shipped, run
157157
> `exchange_and_wrap.sh --database opik --wrap-only` — it runs the settle gate and applies **only** the `RENAME` +
158-
> `Distributed` wrap on the already-EXCHANGEd `traces` (no second EXCHANGE, no new `cutover_start`). To roll the wrap
158+
> `Distributed` wrap on the already-swapped `traces` (no second EXCHANGE, no new `cutover_start`). To roll the wrap
159159
> back, use `rollback.sh --stage C`.
160+
>
161+
> **Wrap flags** (`exchange_and_wrap.sh`, mutually exclusive; default is EXCHANGE-only): omit them (or pass
162+
> `--skip-wrap`, an explicit alias) to run the EXCHANGE and stop; `--with-wrap` to also apply the wrap in the same run;
163+
> `--wrap-only` to apply just the deferred wrap later.
160164
161165
**Dedup note.** After the delta, a row can have two physical versions on `traces_local_v2` (the backfilled one and the
162166
delta one). This is normal — `ReplacingMergeTree` collapses them on merge / under `FINAL` / `LIMIT 1 BY id`, highest

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
# --with-wrap also apply the Distributed wrap in the same run (EXCHANGE + wrap). Use only once the delete/read
2525
# DAOs are sharding-aware. Mutually exclusive with --skip-wrap / --wrap-only.
2626
# --skip-wrap explicit alias for the default (EXCHANGE only); accepted for clarity and back-compat.
27-
# --wrap-only run ONLY the Distributed wrap on the already-EXCHANGEd `traces` (no EXCHANGE, no new cutover_start)
27+
# --wrap-only run ONLY the Distributed wrap on the already-swapped `traces` (no EXCHANGE, no new cutover_start)
2828
# — the deferred second half of a prior EXCHANGE-only run. Mutually exclusive with the above.
2929
# --force skip the replication-settle gate. By default the swap aborts while any replica still
3030
# has replication-queue backlog or an unfinished mutation on traces / traces_local_v2, since a
@@ -103,7 +103,10 @@ assert_pre_exchange_topology() {
103103
[[ -n "$(traces_engine traces_local_v2)" ]] || { echo "ERROR: successor 'traces_local_v2' not found; run the backfill + delta first." >&2; exit 1; }
104104
}
105105

106-
# --wrap-only precondition: traces must be the post-EXCHANGE successor MergeTree (not the original, not already wrapped).
106+
# --wrap-only precondition: traces must be the post-EXCHANGE successor MergeTree (not the original, not already wrapped),
107+
# AND the post-swap RENAME must have completed. In the split state (EXCHANGE done, RENAME not) `traces` already holds
108+
# the successor schema but `traces_local_v2` still holds the old data — wrapping then would orphan the old data under
109+
# the wrong name (finalize.sh would misread it as the disposable successor). So refuse until the rename is finished.
107110
assert_pre_wrap_topology() {
108111
local engine end_time
109112
engine="$(traces_engine traces)"
@@ -117,6 +120,12 @@ assert_pre_wrap_topology() {
117120
echo "ERROR: --wrap-only expects the post-EXCHANGE state (traces = successor schema), but traces has Nullable end_time (the EXCHANGE has not run). Run without --wrap-only first." >&2
118121
exit 1
119122
}
123+
if [[ -n "$(traces_engine traces_local_v2)" ]]; then
124+
echo "ERROR: --wrap-only: 'traces_local_v2' still exists — the post-EXCHANGE RENAME did not complete, so wrapping" >&2
125+
echo " now would orphan the old data under the wrong name. Finish the rename first, then re-run --wrap-only:" >&2
126+
echo " clickhouse-client --database $DATABASE --query \"RENAME TABLE $DATABASE.traces_local_v2 TO $DATABASE.traces_pre_cutover_backup ON CLUSTER '{cluster}'\"" >&2
127+
exit 1
128+
fi
120129
}
121130

122131
# Pre-EXCHANGE gate: the swap is metadata-only and near-instant, but each replica reads its own local parts afterwards.

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ def _handle_sigint(_signum, _frame):
2929
def _fetch_ids(client, project, want, exclude):
3030
try:
3131
traces = client.search_traces(project_name=project, max_results=want, truncate=True)
32-
except Exception as exc: # search is best-effort; keep the loop alive
33-
LOGGER.warning("search_traces failed: %s", exc)
34-
return []
32+
except Exception as exc: # transient search failure: signal the caller to retry, not to treat the pool as drained
33+
LOGGER.warning("search_traces failed (will retry): %s", exc)
34+
return None
3535
return [t.id for t in traces if t.id not in exclude]
3636

3737

@@ -56,7 +56,12 @@ def main(project, tps, duration, batch):
5656
tick = time.time()
5757
if len(pool) < batch:
5858
# Exclude both already-deleted ids and those still queued in `pool`, so a refill can't requeue an in-flight id.
59-
pool.extend(_fetch_ids(client, project, want=500, exclude=seen | set(pool)))
59+
fetched = _fetch_ids(client, project, want=500, exclude=seen | set(pool))
60+
if fetched is None:
61+
# transient search failure — back off and retry rather than mistaking it for "no more traces".
62+
time.sleep(interval if interval > 0 else 0.5)
63+
continue
64+
pool.extend(fetched)
6065
if not pool:
6166
LOGGER.info("no more traces to delete; stopping")
6267
break

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@
5454
"environment",
5555
]
5656

57-
# A far-future instant matching the litellm UUIDv7 bug (ids whose embedded timestamp lands around the year 2201).
58-
BAD_ID_INSTANT = utcnow().replace(year=2201)
57+
# A far-future instant matching the litellm UUIDv7 bug (ids whose embedded timestamp lands around the year 2201). Built
58+
# from a fixed date (not now().replace(year=2201)) so it never hits Feb 29 -> ValueError at import on a leap-day run.
59+
BAD_ID_INSTANT = datetime(2201, 6, 1, tzinfo=timezone.utc)
5960

6061
_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc)
6162
_TAG_POOL = ["prod", "llm", "rag", "eval", "v1", "v2", "canary", "batch", "stream", "agent"]

0 commit comments

Comments
 (0)