Port #3748 to main: executor client connect timeout

[kmatasfp]

Ports #3748 to main.

golem-service-base/src/grpc/client.rs on main was the 1.5.x base plus one addition of its own, call_without_retry on both GrpcClient and MultiTargetGrpcClient, for operations whose request cannot be replayed. That is reinstated on top of the rewritten client, threaded through the new CallAttempts as not_replayable. Its three callers (clients/registry.rs, worker-service's invocation session, worker_proxy.rs) are unchanged.

One behavioural note that falls out of the rewrite: a failed connect is still retried under call_without_retry. Connecting is now eager and happens before the closure runs, so nothing of the request has been consumed and the next attempt is the first one the peer could ever see. Only the call itself is gated.

main's own test for this, multi_target_call_without_retry_attempts_unavailable_request_once, aimed an unreachable target at the client and counted closure invocations. With eager connect that target never reaches the closure at all, so it is replaced by two tests in the new mod test built on its silent_peer harness: one asserting a non-replayable call is attempted exactly once, and its converse asserting the same failure is retried when the request can be replayed, so the first is measuring the gate rather than a client that would not have retried anyway.

The shard-manager config regenerates with three more http2_keep_alive_* keys than the 1.5.x hunks carry, under registry_service and the shard-manager client, both of which differ on main. All twelve config files here are cargo make generate-configs output.

Whole workspace type-checks with --all-targets. golem-service-base: 67 unit tests and the 18 new grpc_client integration tests pass.

[netlify]

✅ Deploy Preview for golemcloud canceled.

Name	Link
🔨 Latest commit	fababb1
🔍 Latest deploy log	https://app.netlify.com/projects/golemcloud/deploys/6aa30c554e0d0c0008213007

[vigoo]

Found one more thing in the new review:

A stream-only HTTP/2 CANCEL reset aborts unrelated RPCs — golem-service-base/src/grpc/client.rs:1156–1157, with cancellation at lines 987–990.

RST_STREAM(CANCEL) produces Code::Cancelled with a tonic::transport::Error source, but leaves the connection and other streams healthy. The new predicate treats it as a dead connection and cancels every pending sibling call. Replayable calls retry unnecessarily; non-replayable calls fail permanently.

Distinguish stream resets from connection failures before cancelling sibling requests. Extend the existing reset test beyond ENHANCE_YOUR_CALM to cover CANCEL and REFUSED_STREAM alongside a pending non-replayable RPC.

Reproduced: a local HTTP/2 server held one real RPC open and reset only the second. The untouched RPC failed with:

Unavailable: connection retired while the request was waiting on it

Reproducer:

diff --git a/golem-service-base/tests/grpc_client.rs b/golem-service-base/tests/grpc_client.rs
index a3e2da8b3..0922f0f7c 100644
--- a/golem-service-base/tests/grpc_client.rs
+++ b/golem-service-base/tests/grpc_client.rs
@@ -1484,3 +1484,40 @@ async fn a_reset_stream_does_not_tear_down_the_connection_carrying_it() {
          new one instead of reusing the channel every other request rides"
     );
 }
+
+#[test]
+async fn review_cancel_reset_must_not_cancel_sibling_call() {
+    let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let uri: Uri = format!("http://{}", listener.local_addr().unwrap()).parse().unwrap();
+    let (started_tx, started_rx) = tokio::sync::oneshot::channel();
+    let server = tokio::spawn(async move {
+        let (socket, _) = listener.accept().await.unwrap();
+        let mut connection = h2::server::handshake(socket).await.unwrap();
+        let first_stream = connection.accept().await.unwrap().unwrap();
+        let _ = started_tx.send(());
+        while let Some(Ok((_request, mut respond))) = connection.accept().await {
+            respond.send_reset(h2::Reason::CANCEL);
+        }
+        drop(first_stream);
+    });
+    let client = executor_client(no_keepalive(Duration::from_secs(5)));
+    let sibling = tokio::spawn({
+        let client = client.clone();
+        let uri = uri.clone();
+        async move {
+            client.call_without_retry("sibling", uri, move |executor| {
+                Box::pin(executor.assign_shards(AssignShardsRequest { shard_ids: vec![] }))
+            }).await.map(|_| ())
+        }
+    });
+    started_rx.await.unwrap();
+    let reset = ping(&client, uri).await.expect_err("peer resets the stream");
+    assert_eq!(reset.code(), tonic::Code::Cancelled);
+    assert!(std::error::Error::source(&reset)
+        .map(|source| source.is::<tonic::transport::Error>()).unwrap_or(false));
+    let mut sibling = sibling;
+    let outcome = tokio::time::timeout(Duration::from_millis(200), &mut sibling).await;
+    sibling.abort();
+    server.abort();
+    assert!(outcome.is_err(), "stream-only CANCEL terminated sibling: {outcome:?}");
+}

[kmatasfp]

Fixed in ee2cd88.

The two predicates read only the code and whether a tonic::transport::Error sat under the status. A reset stream and a dead connection both carry that wrapper, so the code did all the separating, and CANCEL and REFUSED_STREAM land on Cancelled and Unavailable, the same codes a connection closing with requests on it and an expired keep-alive ping arrive as.

What does separate them is further down the chain. Status::from_error keeps the whole transport error as the source, the hyper::Error under it carries the h2::Error, and that one knows it came from RST_STREAM (is_reset()). A connection that dies reaches h2 as an I/O error or a GOAWAY instead. So transport_failed now walks the source chain and answers false for any reset before the code is looked at. worth_reconnecting and connection_gone are unchanged; only their input is. REFUSED_STREAM still evicts the cached connection, through the existing Unavailable-from-anywhere rule, but no longer releases the siblings. h2 moves from dev-dependency to dependency for this; the lock file does not change.

Tests, in tests/grpc_client.rs: a peer that holds the first stream it is sent and resets the rest, and two tests over it for CANCEL and REFUSED_STREAM with a call_without_retry sibling on the held stream. The sibling has to come back Unimplemented once the peer answers it, not "connection retired while the request was waiting on it". The existing ENHANCE_YOUR_CALM connection-count test is also run for CANCEL. All three new cases were red before the fix, with exactly the failure you reproduced.

One note for later: the predicate is identical on 1.5.x, so this needs porting back with the rest.