Repository navigation
Adopt spawned v0.7: ActorSystem shutdown, supervision, test mocks, metrics and a testing roadmap - #783
Draft
unbalancedparentheses wants to merge 8 commits into
Draft
Adopt spawned v0.7: ActorSystem shutdown, supervision, test mocks, metrics and a testing roadmap#783unbalancedparentheses wants to merge 8 commits into
unbalancedparentheses wants to merge 8 commits into
Conversation
Point spawned-concurrency and spawned-rt at the unreleased v0.7 candidate (lambdaclass/spawned fd28d2e9: main 32796a88 plus the spawned-test crate), and declare spawned-test at the same revision so every spawned crate shares one set of types. Mechanical only: ethlambda's 0.5 protocol and handler signatures compile unchanged against v0.7, both crates keep their default features (which now include `async`), and every actor still starts the way it did. Nothing is placed in an ActorSystem yet.
Startup and shutdown were wired by hand: the chain actor on its own `Backend::Thread`, P2P on the shared runtime, and on ctrl-c both were stopped and joined with no deadline. A clean shutdown during catch-up took 14.3 s, past Docker's 10 s grace period (1d7902d), and only SIGINT was handled, so `docker stop` (SIGTERM) killed the node outright unless it was run with `--stop-signal SIGINT`. Now one ActorSystem owns both actors and their threads: - `chain`: a one-worker `thread_group`, the same isolation the chain actor had on `Backend::Thread`: one dedicated native thread for its long synchronous handlers, plus a one-worker Tokio runtime for its timers and the aggregation worker's `spawn_blocking`. - `network`: an `async_group` for P2P, sized like the default Tokio runtime it used to share (one worker under Shadow). The chain actor is admitted before P2P, so the system's reverse-admission shutdown stops P2P, the ingress, before the actor it feeds. discv5 now binds its socket in `run_node` before any actor starts, so a busy port still aborts startup with nothing running; `P2P::spawn` takes the handle and an execution group, `BlockChain::spawn{,_beacon}` take a group. Shutdown answers SIGTERM as well as SIGINT and is bounded by an 8 s deadline: the HTTP server gets 2 s to close (an open SSE stream could otherwise hold it), then the system stops the actors with what is left. A shutdown that misses the deadline logs the unfinished groups and actors by name and exits 1 instead of waiting. Repeated ctrl-c still forces an exit.
Put the HTTP server (API, metrics and debug routes) under a OneForOne supervisor in the actor system. It was a bare `tokio::spawn`: a failure logged one error and the node ran on with no API and no metrics. It is now an `HttpServer` actor, admitted on the `network` group, that serves on an owned helper and stops when the server returns, so the supervisor builds a fresh one. Each run builds its router afresh from cloned handles, which is what makes a restart safe; what one loses is per connection (open event streams, Beacon API fee recipients until the validator client re-registers them). Budget: Permanent, at most 3 restarts a minute, backoff from 1 s to 10 s. A server that keeps failing (its port taken by another process) exhausts it, the supervisor gives up with an error log, and the node runs on without HTTP, as it did before. Restarts and the give-up are logged by a watcher, since spawned logs neither. The tree is admitted last, so shutdown stops it first, with 2 s of the 8 s deadline. The chain and P2P actors stay unsupervised, with the reasons in `run_node`: the chain actor's in-memory state, including the validator keys' prepared signing state, is not rebuilt from the store, and P2P owns the swarm and discv5 sockets built once at startup; both are referenced by the other actors through refs a replacement would not rebind. For test nodes only, `ETHLAMBDA_FAULT_INJECTION=1` makes SIGUSR1 crash the current HTTP server with a real handler panic, to exercise recovery.
The p2p forwarding tests stood in for the chain actor with four hand-written `P2PToBlockChain` impls: `RecordingChain` in column_checks.rs and in test_support, `Recorder` in envelope_checks.rs (also used by envelope_client.rs) and `GloasRecorder` in verdict.rs. Each implemented all eight protocol methods to record one or two of them. They are now `spawned_test::tasks::Mock` actors converted with `to_p2p_to_block_chain_ref()`, so the code under test still holds the protocol ref it holds in production: - `Mock::new().allow_sends()` logs every send; tests read what they care about with `expect` / `take_all` and the arrival order with `received()`. - Tests asserting that nothing reaches the chain actor use a strict `Mock::new()`, which fails on any message at all rather than only the one method the old stub recorded. The stubs were synchronous; a mock is an actor, so tests settle it (or expect the last send) before reading the log. Where the old tests closed a channel to prove nothing followed, the new ones rely on the code's own ordering (the parked batch is the task's last send; the range release fires after the last envelope) and assert the complete log.
Enable spawned-concurrency's `metrics` feature and append its Prometheus text to the existing `/metrics` response: mailbox depth, queue wait, handler duration, request outcomes and actor lifecycle (spawned, exited by reason), all prefixed `spawned_` and summed over every actor in the process. They are what to watch on a canary of the v0.7 runtime: a restarted HTTP server shows as an exit and a new spawn, and a stalled chain actor as queue wait and mailbox depth. Collection is the library's fixed set of atomic counters, timing every message (the default sampling); no exporter or labels are added.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
main, SIGTERM (whatdocker stopsends) has no handler, so the node dies at once with no graceful shutdown; SIGINT exits in 8-11 s. Users were told to raise Docker's stop timeout (1d7902d).P2PToBlockChainrecorder).What changes
chore: upgrade spawned to v0.7refactor: own actors in an ActorSystem with a shutdown deadlineActorSystem: the chain actor in a one-thread group (same isolation asBackend::Thread), P2P in an async group. SIGINT and SIGTERM trigger shutdown with an 8 s deadline; on a miss it names the unfinished groups/actors and exits 1. discv5 binds before any actor starts; P2P (ingress) stops before the chain.feat: supervise restartable actorsETHLAMBDA_FAULT_INJECTION=1+ SIGUSR1 crashes the HTTP server.test: replace hand-written protocol stubs with spawned-test mocksspawned-testmocks; "nothing reaches the chain" tests now fail on any message.feat: export spawned actor metrics on /metricsspawned_*series: mailbox depth, queue wait, handler duration, exits by reason.chore: pin spawned to the v0.7.0 release plus spawned-testspawned-testcrate (lambdaclass/spawned#198).docs: add a testing and verification roadmapdocs/testing_roadmap.md: testing ethlambda the way FoundationDB, TigerBeetle, Dropbox, etcd and AWS test their systems. First step: a seeded in-process chain-logic simulator.Per-method request timeouts were not needed: ethlambda makes no request/reply actor calls.
Measured so far (local)
chain: its first tick replays fork choice slot by slot from genesis (existing behaviour; checkpoint-synced nodes avoid it)Fault injection on the real binary: HTTP server back in 0.17-0.28 s,
/metricsanswering again.A canary on a dedicated server is running now; results will be posted here.
Reading guide
bin/ethlambda/src/main.rs).docs/testing_roadmap.md.Known follow-ups
_maxexposition bug) is tracked in spawned.