-
[OPIK-7168] [SDK] feat: resume interrupted opik migrate from a local checkpoint (#7358)
frostbyte_neo 发布于
2026-07-08 13:22:59 +00:00 - [OPIK-7168] [SDK] feat: resume interrupted opik migrate from a local checkpoint
Add per-experiment checkpoint/resume to
opik migrate datasetso an
interrupted run (crash, network drop, OOM) resumes from the last completed
experiment instead of restarting.- New MigrationCheckpoint store: keyed by workspace+project+dataset, written
adjacent to the audit log, atomically flushed after every completed
experiment so it survives an uncatchable OOM SIGKILL. Corrupt/foreign-schema
files start fresh; deleted on full success. - Cascade skips already-completed experiments and seeds the progress bar so a
resumed run renders at the right percentage, not 0. - An experiment interrupted mid-flight has its partial destination data
(traces -> spans cascade on the BE, plus a possibly-orphaned experiment row)
deleted before it is re-migrated, so re-runs leave no duplicates. The
backend does not cascade-delete on re-migrate, so the SDK does it client-side. - Unit tests cover resume, skip-completed, re-migrate-incomplete, progress
seeding, lifecycle, and corrupt-file tolerance. Docs updated.
Implements OPIK-7168.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- fix(migrate): close resume-cleanup gaps from PR review
Address Baz review findings on the checkpoint/resume path:
- Record + flush destination trace ids to the checkpoint INSIDE
_copy_traces_and_spans, before the backend flush that persists them, so a
crash anywhere from the trace flush through the span-copy phase leaves the
ids recorded for the next run's cleanup (previously recorded only after the
copy returned, leaving that window uncovered → duplicates on resume). - Delete the interrupted experiment's destination row by a recorded
dest_experiment_id instead of matching by name. Experiment names aren't
unique in the destination dataset, so a name match could delete a peer. The
id is minted in the cascade and checkpointed before the row is created;
create_experiment/recreate_experiment gain an optional experiment_id. - Harden load_or_create against a malformed in_flight payload (falls back to
fresh instead of raising KeyError/TypeError), matching the existing
corrupt-JSON / foreign-schema recovery.
Tests updated for delete-by-id, no-experiment-row-yet, before-backend-write
ordering, malformed-in_flight, and dest_experiment_id round-trip.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- fix(migrate): validate checkpoint id lists + document experiment_id
Follow-up to PR review on the previous fix commit:
- load_or_create now validates that completed_experiment_ids and the in-flight
dest_trace_ids are lists of strings. set()/list() silently split a bare
string into characters without raising, so a corrupt value would seed the
wrong completed set and make the cascade skip or re-run the wrong
experiments; such shapes now fall back to a fresh checkpoint. - Document the new experiment_id arg in recreate_experiment's Args block,
matching its fully-documented siblings.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- test(migrate): add E2E resume test with deterministic mid-cascade crash
The unit tests cover the resume logic against a stand-in client; this adds
the missing end-to-end coverage for the real crash→resume round-trip against
a live backend — the scenario the feature exists for.- New test-only fault-injection hook in the cascade: when
OPIK_MIGRATE_CRASH_AFTER_EXPERIMENT=N is set, the process os._exit(137)s
(uncatchable, OOM-like) partway through experiment N, after its traces are
written + checkpointed but before the experiment row is created. No-op in
production (single os.environ.get when unset). - run_migrate_cli gains an extra_env param to inject that flag into the child.
- test_migrate_dataset_resume_e2e: seed a multi-experiment source, crash
mid-cascade, assert the checkpoint survived with partial progress, then
re-run and assert the destination has exactly one experiment per source
(no duplicates — find_destination_experiment raises on >1), the interrupted
experiment's partial traces were cleaned up, and the checkpoint is deleted.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- feat(migrate): resume dataset-phase-complete migrations without duplicating
End-to-end resume was broken: on re-run the plan re-emitted RenameSource ->
CreateDestination -> ReplayVersions, but the first run already did all three,
so resume collided on the rename ( already renamed to _v1, destination
already created) and never reached the resumable experiment cascade. Even past
the collision, ReplayVersions' version_remap/item_id_remap live only in memory
and the cascade can't rewrite experiment FKs without them.Fix — resume from a completed dataset phase:
- Checkpoint records dataset_phase_done + the source dataset id and its
post-rename name, set once the cascade first starts on a fresh run. - On resume, build_dataset_plan emits a cascade-only plan: it resolves the
source as _v1 (the original name now points at the destination) and
rebuilds version_remap/item_id_remap/optimization_id_remap READ-ONLY from the
already-migrated destination (reconstruct_remaps) — versions pair by
chronological index, item ids are identity (backend copy_from preserves
dataset_item_id), optimizations match by name. Nothing is re-written, so no
versions are duplicated. - A version-count mismatch (e.g. crash mid-replay) fails loud with a
delete-and-restart message rather than mis-pairing.
The user re-runs the exact same command; the tool detects the resume from the
checkpoint. Verified end-to-end against an ad-hoc PR env: crash at experiment
5/9, re-run -> 9 destination experiments, no duplicates, interrupted
experiment's partial traces cleaned up. Unit tests cover the reconstruction
pairing/identity/fallback/mismatch paths; the E2E test asserts the full
crash->resume round-trip.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- fix(migrate): tolerate a non-integer crash-injection env value
_maybe_crash_for_resume_test called int() on
OPIK_MIGRATE_CRASH_AFTER_EXPERIMENT directly, so a non-integer value would
raise ValueError and abort a real migration mid-cascade. Treat an unparseable
value as unset (no-op), matching the hook's existing unset contract — a
test-only knob must never crash a real run.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- chore: drop throwaway manual test seed script from PR
manual_test_seed_opik_7168.py is a local manual-test helper (seeds the ad-hoc
env for the crash/resume test); it was swept in by git add -A and doesn't
belong in the SDK. Keep it untracked.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- fix(migrate): close resume-flag gaps from PR review
- Mark dataset_phase_done right after CascadeOptimizations completes (in
_apply_action), not at the start of _cascade_experiments. Closes the window
where a crash between the optimizations action and the cascade boundary left
the flag false, sending the next run down the full-plan branch and colliding
on the already-applied rename. - load_or_create now requires dataset_phase_done to be a real bool; a truthy
non-bool (e.g. the string "false", a list) falls back to a fresh checkpoint
instead of forcing a resume plan that skips rename/create/replay. Matches the
strict validation on the id-list fields. - Rename the reconstruct tests to include the expected-result suffix per the
python-sdk testing convention.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- refactor(migrate): address human review — checkpoint location + test-only crash seam
Two items from @alexkuzmik's review:
- Checkpoint now lives at a fixed ~/.opik/migrate-checkpoints/ (keyed by
workspace+project+dataset), independent of cwd/--audit-log. Previously it sat
next to the audit log (cwd), so re-running from a different folder found no
checkpoint and silently restarted — resume was broken for that case. - The test-only crash-injection hook is gone from production experiments.py
(removed _maybe_crash_for_resume_test, its env var, the processed_index param,
and the os import). The E2E resume test now injects the deterministic crash
via a test-owned sitecustomize.py on the subprocess PYTHONPATH, which also
redirects the checkpoint dir into tmp_path — no test-only logic in the product.
A globally-robust checkpoint location (CI ephemeral HOME, container-OOM needing
a mounted volume, OPIK_MIGRATE_CHECKPOINT_DIR override) is tracked as a follow-up.397 unit tests pass; ruff + mypy clean.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
- fix(migrate): checkpoint I/O is non-fatal + E2E test hardening
Address PR review on the ~/.opik checkpoint move:
- A checkpoint is only a resume aid, so it must never abort a healthy
migration. load_or_create now returns None when the location can't be
resolved (e.g. Path.home() raises on a homeless CI/container), and flush()
swallows OSError (read-only / full disk) with a warning — the run continues,
just not resumable from there. The CLI treats None as "no resume support". - E2E: wrap the post-resume destination reads in synchronization.until so
eventual-consistency lag doesn't flake the assertion; derive seeded
experiment names from the dataset_name fixture instead of random_chars.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Co-authored-by: Claude Opus 4.8 noreply@anthropic.com
下载附件