Introduce session persistence and artifact caching
This change implements a robust session management system allowing users to persist distance calculation samples and parameters across invocations. It includes concurrency-safe directory locking, deterministic parameter encoding, artifact integrity checks, and caching mechanisms to skip resampling when valid data is available.
This commit is contained in:
@@ -2715,38 +2715,99 @@ sample)` and the `snp_distance` stage timer reads `0ms` (`formula(kind)`
|
||||
is `O(n²)` post-processing, no I/O), vs. ~150-230ms for a fresh sample on
|
||||
the same tiny test index.
|
||||
|
||||
#### Future direction, not implemented: explicit `--session`
|
||||
#### Explicit `--session` (implemented 2026-09-12)
|
||||
|
||||
Raised in discussion, not started. The fix above only covers reuse
|
||||
*within one process*. A further idea: a `--session DIR` flag naming a
|
||||
directory (outside the index) that persists the CLI selection parameters
|
||||
plus every intermediate artifact `sample_index` would otherwise
|
||||
recompute — the site set itself, `PairwiseTally`/`PartitionDispersion`,
|
||||
the pseudo-alignment, etc. — across *separate* `obikmer phylo`
|
||||
invocations, e.g. running `--sankoff` today and `--distance snp-k2p`
|
||||
tomorrow against the identical sample. Without `--session`, an implicit
|
||||
*temporary* session would still be created (scoped to the index directory,
|
||||
matching the tmp-cache design discussed earlier in this file's α-estimation
|
||||
section) — same mechanism, just not named/kept by the user.
|
||||
The in-process fix above only covered reuse *within one command*.
|
||||
`--session DIR` (`cmd/phylo/args.rs`) names a directory (outside the
|
||||
index) that persists the `snp-*` `--distance` sample — the
|
||||
`PairwiseTally`/`PartitionDispersion` pair `sample_index` would otherwise
|
||||
rebuild from scratch — across *separate* `obikmer phylo` invocations.
|
||||
|
||||
Two open design points if this is picked up:
|
||||
- **Cache-key validity**: as established for the tally cache above, the
|
||||
content depends on `n`/`free_loss`/`no_ambiguity`/excluded-set/
|
||||
`entropy_bias` — a session is only reusable for the exact tuple it was
|
||||
built under. `--subsample` in particular is the parameter most likely to
|
||||
change between exploratory runs, so an *exhaustive* (`n = None`) session
|
||||
that later runs subsample *from*, rather than one session per exact `n`,
|
||||
would likely see far more real reuse.
|
||||
- **Concurrency-safe cleanup**: `obisys::DirLock` (`lock.rs`) is the
|
||||
existing pattern to follow — an OS advisory lock (`flock`/`LockFileEx`),
|
||||
auto-released on process exit *including a crash*, no stale-lock cleanup
|
||||
logic needed. Applied per cache/session entry (not just once for the
|
||||
whole index as `DirLock` does today for annex writes): a process
|
||||
deciding whether to reclaim an old temporary session first tries to
|
||||
acquire that entry's lock — success means nobody's using it, safe to
|
||||
delete; failure means another process holds it, leave it alone. Avoids
|
||||
the hazard of one process deleting another concurrently-running
|
||||
process's temp session, which a naive "wipe at startup" would risk.
|
||||
**Crate placement, as planned**: a new **`obiksession`** crate
|
||||
(`src/obiksession/`), domain-agnostic — no `PairwiseTally`/"site" concept
|
||||
anywhere in it. `Session::open(dir, params: &[u8], force: bool)` handles
|
||||
directory lifecycle and the params-conflict check against an opaque byte
|
||||
blob; `Session::store(name, bytes)`/`Session::restore(name) -> Option<Mmap>`
|
||||
handle a checksummed artifact cache. `obikphylo`
|
||||
(`siblings/algorithms/snp_distance.rs`) is the consumer: it serializes its
|
||||
own `PairwiseTally`/`PartitionDispersion` to bytes (via `rkyv`) and hands
|
||||
them to `obiksession`, which never sees their type.
|
||||
|
||||
**Serialization**: `rkyv` 0.8.18, added as planned. One deviation from the
|
||||
original zero-copy pitch, scoped down deliberately: artifacts are restored
|
||||
via `rkyv::from_bytes` (full owned deserialize) rather than `rkyv::access`
|
||||
(zero-copy over the `mmap`). True zero-copy would require every
|
||||
`PairwiseTally` query method (`categories`, `base_freq`, `pair`, ...) to
|
||||
work generically over `Archived<PairwiseTally>` as well as the owned type
|
||||
— a separate, larger change not needed to get the actual win (skipping
|
||||
`sample_index`'s expensive re-scan of the sibling annex; deserializing an
|
||||
already-in-memory-sized count array is comparatively cheap CPU, not I/O).
|
||||
`#[derive(Archive, Serialize, Deserialize)]` was added to `PairStats`,
|
||||
`PairwiseTally`, and `PartitionDispersion` (`pairwise.rs`) for this.
|
||||
|
||||
**Startup behavior, as designed**: `Session::open` creates `DIR` if
|
||||
missing (fresh, params recorded); if `DIR` already holds different saved
|
||||
params, `cmd/phylo` reports a hard error and exits rather than silently
|
||||
preferring one side, unless `--session-force` is given (which discards the
|
||||
directory's cached artifacts and starts over under the new params) —
|
||||
implements exactly the "explicit escape hatch, not silent override"
|
||||
decision from the design discussion. `obiksession` itself doesn't decode
|
||||
either side's bytes to produce a field-by-field diff (true to "opaque
|
||||
blob," left to whoever needs it); `cmd/phylo`'s conflict message is
|
||||
currently a single generic sentence listing which flags could be the
|
||||
cause, not a computed diff — an honest v1 simplification, not a limitation
|
||||
of `obiksession` itself.
|
||||
|
||||
**Per-artifact caching, as designed**: only `PairwiseTally`/
|
||||
`PartitionDispersion` are cached (`snp_distance.rs`'s `restore_tally`/
|
||||
`store_tally`) — `ratio_ceiling`/`gamma_shape` remain pure post-processing
|
||||
over whatever tally is in hand, cached or fresh, exactly as planned.
|
||||
|
||||
**Locking**: `obiksession::Session` holds one `obisys::DirLock` for the
|
||||
*whole session directory*, for the `Session` value's entire lifetime — not
|
||||
the per-artifact-entry locking originally sketched. Simpler, and still
|
||||
fully correct/crash-safe (same OS-auto-release guarantee `DirLock` already
|
||||
provides): a second `obikmer phylo` process pointed at the same
|
||||
`--session DIR` blocks until the first releases it, rather than each
|
||||
artifact being independently lockable. Verified end-to-end (throwaway
|
||||
8-genome index): same params → second run restores in `0ms` vs. the first
|
||||
run's `~150-200ms`, output matrices identical; different params without
|
||||
`--session-force` → clean error, process exits; with `--session-force` →
|
||||
cache discarded, fresh sample computed and stored under the new params.
|
||||
|
||||
**Explicitly not done in this pass** (each a real follow-up, not silently
|
||||
dropped):
|
||||
- **Per-layer chunked/resumable dumps — genuinely missing, not a
|
||||
justified tradeoff.** Each artifact is a single store/restore of the
|
||||
whole tally; a crash mid-`sample_index` loses the *entire* in-progress
|
||||
call, however far it had gotten. An earlier pass at this note argued the
|
||||
gap was fine because "the real cost is the I/O scan, not the tally
|
||||
serialization, and redoing the tally is cheap" — that's a non sequitur,
|
||||
caught and corrected on review: the cost that matters for resumability
|
||||
is however much of the *scan* was already done when it died, not how
|
||||
cheap the final tally is to serialize. That argument was extrapolated
|
||||
from an 8-genome throwaway smoke test (~150-200ms total), not verified
|
||||
against the actual scale this project targets (hundreds of partitions,
|
||||
potentially hours — see the `merge_partitions` runs discussed earlier in
|
||||
this project). On a long real run, losing 90% of a completed scan to a
|
||||
crash is exactly the expensive case per-layer checkpointing exists to
|
||||
avoid, and nothing here measures whether that's rare or common in
|
||||
practice. Left undone for the same reason as the `SankoffBundle` gap
|
||||
below (time, not a technical blocker) — not because the resumability
|
||||
question was settled in favor of skipping it.
|
||||
- **`SankoffBundle` integration.** `--session` only wires into the
|
||||
standalone `snp_distance()` path today. `--sankoff`/`--tnt`/`--phyg`/
|
||||
`--iqtree` still always resample, even with `--session` given — their
|
||||
`SnpAlignment`/`BasePairTally`/`CardinalityTally` aren't cached. Same
|
||||
mechanism would extend to them; not done yet.
|
||||
- **Exhaustive-sample caching for `--subsample`-varying exploration.** The
|
||||
design discussion's suggestion — cache the exhaustive (`n = None`)
|
||||
sample once and subsample *from* it on each run instead of one session
|
||||
per exact `n` — wasn't implemented; a session today is still scoped to
|
||||
one exact parameter tuple, `--subsample` included.
|
||||
- Only the whole-session lock (see above), not the finer per-entry
|
||||
locking originally sketched for reducing contention between processes
|
||||
sharing one `--session` for *different* artifacts.
|
||||
|
||||
### Output format: PHYLIP-relaxed by default for the distance matrix
|
||||
|
||||
|
||||
Reference in New Issue
Block a user