Arc-ify stored log text to remove per-line double clones (ticket 100)
DataValue::LogLiteral and IngestedLine now carry an Arc<str> instead of an owned String, so the two clones the ingest path paid per line (once to move text into the write closure, once to store it) become cheap refcount bumps instead of full string copies. Also cuts the per-string-content cost of left_right's full-EngineSnapshot deep clone on each reconciliation sync. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyhRfMYjMditek6geDRoQ
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
5fee6909f2
commit
f43445b280
@@ -0,0 +1,107 @@
|
||||
# 102 — Split the single engine actor into reader, primary, and profile actors
|
||||
|
||||
## Summary
|
||||
|
||||
Today, one actor (`run()` in `src/engine/handle.rs`) does everything:
|
||||
tailing/ingest bookkeeping, raw log storage, param extraction, *every*
|
||||
declared metric's regex evaluation, sort-helper indexing, eviction, and
|
||||
alert evaluation, all serialized through one `mpsc` channel. This is
|
||||
migration step 3 (the actual split) of [[0001-actor-split]]'s resolved
|
||||
plan, and depends on [[100-arc-ify-stored-log-and-metric-strings]] and
|
||||
[[101-extract-metric-computation-into-separate-stage]] landing first —
|
||||
this ticket is the wiring/topology change those two steps prepare for,
|
||||
not a from-scratch redesign. See [[0001-actor-split]]'s "Resolved
|
||||
decisions" section for the full rationale and answers to the open
|
||||
questions below; this ticket's scope is that design, made concrete.
|
||||
|
||||
## Where
|
||||
|
||||
- `src/engine/handle.rs:130-365` (`run()`) — the single actor loop to
|
||||
be split. The `EngineMessage::Ingest` arm (:143-219) is the part that
|
||||
becomes the reader→primary boundary; `apply_all_metrics` /
|
||||
`apply_metric_to_log` (`src/engine/snapshot/mod.rs:616`, :499-586)
|
||||
and alert evaluation (`src/engine/alerts.rs:211`, called from
|
||||
`handle.rs:199-219`) are what moves to the new profile actor.
|
||||
- `src/cli/mod.rs:95` (`spawn_engine`) and `:105-124`
|
||||
(`load_active_profile`) — the sole production call site of
|
||||
`engine::spawn`; `load_active_profile`'s existing comment ("there's
|
||||
no way to reprocess earlier lines later, so this must happen before
|
||||
`spawn_ingest`", `:104-106`) documents a constraint the new topology
|
||||
must preserve for the primary actor's startup ordering.
|
||||
- `src/engine/ingest.rs` (stdin/file-tailing + Prometheus-polling
|
||||
tasks, spawned at `src/cli/mod.rs:136-145`) — today's only
|
||||
multi-task precedent, producer-side only (multiple producers into one
|
||||
actor's channel); the new reader actor sits upstream of this or
|
||||
subsumes part of it, per [[0001-actor-split]]'s three-actor design.
|
||||
- `src/api/mod.rs:712-768` (`activate_profile` /
|
||||
`reload_engine_declarations`) — the runtime profile-switch path that
|
||||
currently removes-then-redeclares against the single actor; needs to
|
||||
target the new profile actor instead. See [[0004-profile-switch-reindexing-cost]]
|
||||
for the existing cost analysis of this path, which doesn't disappear
|
||||
with this split but moves and stops blocking the primary.
|
||||
- `src/engine/tests.rs` (2348 lines) and the `test_handle` pattern
|
||||
(see ticket 099's removal of the duplicate `test_handle` stub) —
|
||||
built around one actor with one channel; will need real rework, not
|
||||
just extension, to cover primary alone, profile actor alone, and
|
||||
primary+profile interaction.
|
||||
|
||||
## Scope
|
||||
|
||||
Implement the three-actor topology exactly as resolved in
|
||||
[[0001-actor-split]]:
|
||||
|
||||
- **Reader actor**: tails stdin/file, splits into lines, timestamps
|
||||
them, wraps each line in an `Arc` (per [[100-arc-ify-stored-log-and-metric-strings]]),
|
||||
sends downstream.
|
||||
- **Primary actor**: ingests timestamped `Arc` lines, extracts declared
|
||||
+ auto params only (no metrics, no profile logic), maintains the
|
||||
`param=value` index. Owns retention/eviction (`SetMaxLogs`) on its
|
||||
own schedule — a lagging profile actor does not block primary
|
||||
eviction; it silently misses data for the evicted range (accepted
|
||||
tradeoff, not a bug to fix here).
|
||||
- **Profile actor**: holds its own `left_right` reader over the
|
||||
primary's index; builds metrics, custom params, timeseries, and
|
||||
PromQL evaluation for the one active profile. Owns alert evaluation
|
||||
(moved from the primary per the resolved decision that alerts are
|
||||
profile-scoped). Still exactly one active profile at a time — no
|
||||
concurrent multi-profile indexing; this changes *how* the active
|
||||
profile's data is computed, not the single-active-profile user model.
|
||||
- **Primary→profile signaling**: a `tokio::sync::watch` (or equivalent)
|
||||
that the primary notifies right after each `left_right` sync
|
||||
completes; the profile actor wakes on that signal and re-reads
|
||||
through its own `left_right` reader — no busy-polling, no second data
|
||||
copy in transit.
|
||||
- **Query fan-out**: a layer for requests that need both raw-log and
|
||||
profile-derived data in one response (e.g. `Logs`/`Timeline`
|
||||
queries, which today read both from a single `EngineSnapshot` in one
|
||||
pass — see `query.rs`'s `log_entry_params` call against
|
||||
`compiled_params`) — this becomes a real join across two
|
||||
snapshots/actors.
|
||||
- **`EngineHandle` API**: split into two handle types (primary vs.
|
||||
profile), or one combined handle that internally fans out — decide
|
||||
during implementation which `api.rs`'s `ApiState` should hold; either
|
||||
is acceptable per [[0001-actor-split]], this ticket doesn't mandate
|
||||
one.
|
||||
- Rework `src/engine/tests.rs` and the `test_handle` pattern to cover
|
||||
the new topology (primary alone, profile actor alone,
|
||||
primary+profile interaction) — explicitly in scope, not deferred,
|
||||
since the existing single-actor test harness cannot exercise the new
|
||||
split at all.
|
||||
- Update `src/api/mod.rs:712-768`'s `activate_profile` /
|
||||
`reload_engine_declarations` to address the profile actor.
|
||||
|
||||
Out of scope (per [[0001-actor-split]]'s resolved decisions —
|
||||
explicitly deferred, not silently dropped):
|
||||
|
||||
- System param suppressions — dropped from scope entirely; revisit as
|
||||
a separate idea if it becomes a real requirement.
|
||||
- Full-text/regex indexing on the primary — a materially different
|
||||
investment ([[0005-additional-read-indexes]]), doesn't depend on
|
||||
actor topology.
|
||||
- Profile-actor crash/restart checkpointing beyond "restart and
|
||||
rebuild from whatever the primary currently retains" — no persisted
|
||||
checkpoint required for this ticket.
|
||||
- Diff-based or async profile-switch reload
|
||||
([[0004-profile-switch-reindexing-cost]]'s options) — this ticket
|
||||
only needs `activate_profile` to target the right actor, not to
|
||||
change the switch's synchronous-rebuild cost model.
|
||||
Reference in New Issue
Block a user