Add durable streaming recovery benchmarks by vigoo · Pull Request #3966 · golemcloud/golem

vigoo · 2026-09-24T16:23:45Z

Summary

add trusted invocation-session tooling and deterministic fixture APIs for reconnect, cold reconstruction, and hard-crash stream recovery
propagate executor-authoritative terminal cursor metadata through invocation acceptance so proxies and clients validate resumed terminal streams consistently
add benchmark infrastructure for structured phase failures, lifecycle kill/reap controls, labelled storage-operation metrics, PostgreSQL session-index inspection/deletion, and SHA-256 artifact provenance
add warm-history, cold-indexed, missing-index rebuild, reconnect, flat recovery, sibling recovery, and nested recovery benchmarks with correctness checks
register the dedicated crash-recovery integration target and all streaming benchmarks in smoke, quick, and daily suites

Verification

cargo test -p golem-api-grpc --lib -- invocation_session_protocol::tests --report-time (47 passed)
cargo test -p golem-worker-executor --lib -- durable_stream_index_reads_are_bounded durable_stream_missing_index_rebuilds_once logical_storage_counters --report-time (4 passed)
cargo test -p integration-tests --lib -- benchmark_registry_resolves_streaming_suites streaming_history::tests streaming_recovery::tests --report-time (7 passed)
cargo test -p golem-test-framework --lib -- live_inspection live_session_index --report-time (2 passed)
scoped cargo clippy for golem-test-framework, integration-tests, and golem-worker-executor with -D warnings
spawned smoke runs for B–H and dedicated hard-crash recovery suite (4/4)
benchmark-results append, tests, production build, and regression analysis accept the generated provenance JSON

Resolves GOL-552

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

…ures Verify worker-service reconnect, exact-cursor Takeover, asymmetric siblings, and second-restart session retirement. Include nested mid-production regression coverage exposing terminal resume cursor propagation. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d328-73c0-7761-a589-2e04adc193e3 Co-authored-by: Amp <amp@ampcode.com>

Initialize all private protocol validators from executor-verified terminal cursors. Require unique requested output offsets, preserve closed roots across Takeover, and continue requiring child output terminals. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d328-73c0-7761-a589-2e04adc193e3 Co-authored-by: Amp <amp@ampcode.com>

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0d380-a61a-70a1-b5a9-cef0af93ffa0

Keep domain, environment, application, and owning account cleanup best effort, recording every real failure. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d380-a61a-70a1-b5a9-cef0af93ffa0 Co-authored-by: Amp <amp@ampcode.com>

Measure trusted resume and takeover phases with deterministic gated prefixes, physical older-session index checks, exact suffix validation, and public retirement observations. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d380-a61a-70a1-b5a9-cef0af93ffa0 Co-authored-by: Amp <amp@ampcode.com>

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

netlify · 2026-09-24T16:23:54Z

✅ Deploy Preview for golemcloud canceled.

Name	Link
🔨 Latest commit	`eb6c26c`
🔍 Latest deploy log	https://app.netlify.com/projects/golemcloud/deploys/6ab67d99ca309a000827a682

vigoo · 2026-09-25T08:22:40Z

  repeated string command_path = 12;
+  // Requested output cursors verified against durable history to already name a terminal.
+  // No suffix or repeated terminal is sent for these streams on this attachment.
+  repeated golem.common.UUID terminal_cursor_stream_ids = 14;


Why did we have to extend the protocol?

The nested recovery case exposed a production protocol gap, not a benchmark-only need. On reconnect the public client sends opaque cursor tokens through the stateless worker-service proxy. Only the executor, after reading authoritative durable state, knows which requested output streams had already terminated. Without carrying that verified set in InvocationAccepted, the proxy starts with fresh state and waits for already-delivered root terminals forever (or would have to guess and risk suppressing unfinished child suffixes). terminal_cursor_stream_ids carries only executor-verified, requested output streams; shared protocol validation rejects duplicates, unrequested/input/start IDs, and initializes those streams as closed. This is what makes nested public reconnect sound.

vigoo · 2026-09-25T08:22:51Z

        run: |
          cargo run --profile benchmarks -p integration-tests --bin benchmarks '--' \
            --primary-only \
+            --retain-details \


Why do we need these new parameters?

They made failure detail and selected zero-count series survive --primary-only, but that changes the existing publisher workflow and is not necessary for the benchmark scenarios themselves. I removed both workflow arguments in 9adb4e2c.

vigoo · 2026-09-25T08:25:04Z

        self.blocking_kill();
    }

+    async fn kill_and_wait(&self, deadline: tokio::time::Instant) -> anyhow::Result<()> {


How is this more than the existing kill which wasl also blocking on the process kill?

The old kill called Child::kill, immediately dropped the child handle, and never observed process exit. That can race a restart and can leave an unreaped child. kill_and_wait owns the lifecycle lock, sends the kill, polls try_wait until the shared absolute deadline, retains the child handle on timeout so it can be reaped later, and reports failure instead of silently continuing. Per your follow-up, 8a48f778c removes the weaker executor kill/kill_all APIs and converts all callers to the bounded variants.

vigoo · 2026-09-25T08:25:58Z

-                return;
-            }
-        }
        let decision = if let Some(deadline) = recovery.deadline {


How is this change related to benchmarks?

It is related because the nested benchmark reproduced a real reconnect failure through the public worker-service WebSocket path: terminal root streams were known to the executor but not to the stateless proxy. The earlier local HTTP-driver initialization was only a partial workaround. The final implementation removes that local special case; the executor now returns the authoritative terminal-stream set in acceptance and the shared session state initializes from that one contract, so gRPC proxy and HTTP/public clients behave consistently.

vigoo · 2026-09-25T08:26:21Z

    fn agent_id() -> Option<AgentId> {
        Some(AgentId {
-            component_id: None,
+            component_id: Some(golem_api_grpc::proto::golem::component::ComponentId {


Why are we returning a fake component id here?

This is inside the protocol-test mock only. The tested conversion requires a ComponentDto, but the component identity is irrelevant to the acceptance/stream-state behavior under test, so the fixture supplies a dummy ID rather than creating registry state. No production path returns this ID.

vigoo · 2026-09-25T08:27:42Z

+
+pub type BenchmarkRegistry = BTreeMap<&'static str, BenchmarkRunFn>;
+
+pub fn benchmark_registry() -> BenchmarkRegistry {


What is this replacing and why was it needed?

It extracted the registration table that previously lived in the benchmark binary so a temporary direct smoke runner could reuse it. That was scaffolding for development, not a production requirement. I removed the standalone module in 8a48f778c; registration is owned by all.rs again.

vigoo · 2026-09-25T08:29:03Z

+        "streaming-rpc-history"
+    }
+
+    fn description() -> &'static str {


This description is not really easy to understand. Take a look at the existing benchmark descriptions. We need something that explains what our benchmark scenario is and what is measured.

Rewritten. The descriptions now state the exact setup, what size and length mean, where the restart/disconnect occurs, which timings and counters are measured, and which setup work is excluded. The description rewrite is in c52ee8dcb, with the public-WebSocket wording updated by 8a48f778c.

vigoo · 2026-09-25T08:30:14Z

+    }
+}
+
+pub struct InvocationSession {


What is this, and why are we not using the streaming client of golem-client?

Agreed. The benchmarks now use golem_client::invocation_session::InvocationSession over the real authenticated public worker-service WebSocket endpoint for Start, Resume, and Takeover. The new code is only a thin benchmark observation adapter around that client so it can record milestone times and validate returned values; reconnect/cursor/protocol handling remains in golem-client. The private driver remains only in the dedicated protocol integration tests. Implemented in 8a48f778c.

vigoo · 2026-09-25T08:30:34Z

+type Call = (&'static str, &'static str, &'static str);
+
+#[derive(Debug)]
+pub(crate) struct CountingKeyValueStorage {


Why do we need o count KV store operations?

This wrapper existed only to make exact in-process assertions for the bounded/missing-index unit tests added with the benchmarks. It did not prove a previously reported production issue and was unrelated to the requested streaming benchmark behavior. I removed the wrapper and those tests in 8a48f778c. The benchmarks still record the production logical storage metrics.

vigoo · 2026-09-25T08:31:02Z

+    pub sessions_present: BTreeSet<IdempotencyKey>,
+}
+
+pub async fn inspect_live_session_index(


Explain why do we need this machinery

This was benchmark-specific PostgreSQL fault-injection machinery: stop/reap executors, inspect the private session-index rows, delete exactly that namespace while proving oplog/payload/KV state was unchanged, then restart so the executor rebuilt derived AgentStatusRecord/session-index state by folding the oplog. It enabled a synthetic “missing index rebuild” scenario that public APIs cannot create. A healthy cold-index benchmark does not need it: it can restart normally, make the first public invocation, and use logical storage metrics to detect history-sized regressions. Since rebuilding deliberately deleted derived storage is outside the streaming-benchmark scope, I removed this helper and the cold-rebuild benchmark in 8a48f778c.

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

…chmarks

Populate the terminal_cursor_stream_ids field added to the durable streaming protocol on the main-side rewritten RPC acceptance path (a fresh start carries no output cursors, so the list is empty), and pass the session callee fingerprint to lookup_durable_stream_control_metadata in the session index tests, matching the fingerprint-scoped signature. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d842-615a-746d-a864-29d8de10614c Co-authored-by: Amp <amp@ampcode.com>

…chmarks Amp-Thread-ID: https://ampcode.com/threads/T-01a0d842-615a-746d-a864-29d8de10614c Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # golem-worker-executor/src/services/worker/session_index_tests.rs

vigoo and others added 12 commits September 24, 2026 15:32

Add streaming benchmark foundation

ad8afbd

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Register streaming recovery tests and benchmarks centrally

835caaf

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Preserve benchmark diagnostics and artifact provenance

0f126ae

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Add streaming history and cold reconstruction benchmarks

b0684e0

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Add read-only live session index inspection for reconnect benchmarks

64e643e

Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0d380-a61a-70a1-b5a9-cef0af93ffa0

Register streaming recovery benchmark suites

7daf8a8

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Fix streaming benchmark provenance and read guard

19da5c9

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Adapt streaming benchmarks to fingerprinted storage

60b43d0

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

vigoo requested a review from a team September 24, 2026 16:23

vigoo commented Sep 25, 2026

View reviewed changes

vigoo and others added 6 commits September 25, 2026 09:00

Clarify streaming benchmark descriptions

c52ee8d

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Address streaming benchmark review feedback

8a48f77

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Keep benchmark workflow output unchanged

9adb4e2

Amp-Thread-ID: https://ampcode.com/threads/T-01a0d300-a45d-754b-9fb1-373a5f1f640a Co-authored-by: Amp <amp@ampcode.com>

Merge remote-tracking branch 'origin/main' into gol-552-streaming-ben…

8d47056

…chmarks

Merge remote-tracking branch 'origin/main' into gol-552-streaming-ben…

eb6c26c

…chmarks Amp-Thread-ID: https://ampcode.com/threads/T-01a0d842-615a-746d-a864-29d8de10614c Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # golem-worker-executor/src/services/worker/session_index_tests.rs

vigoo merged commit 0816546 into main Sep 25, 2026
69 checks passed

vigoo deleted the gol-552-streaming-benchmarks branch September 25, 2026 14:31

github-actions Bot locked and limited conversation to collaborators Sep 25, 2026


		pub type BenchmarkRegistry = BTreeMap<&'static str, BenchmarkRunFn>;

		pub fn benchmark_registry() -> BenchmarkRegistry {

Conversation

vigoo commented Sep 24, 2026

Summary

Verification

Uh oh!

netlify Bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for golemcloud canceled.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vigoo Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vigoo Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

netlify Bot commented Sep 24, 2026 •

edited

Loading

vigoo Sep 25, 2026 •

edited

Loading

vigoo Sep 25, 2026 •

edited

Loading