Symptom
TestForwarding_Progress_RealBackend (pkg/vmcp/server/forwarding_realbackend_integration_test.go) intermittently fails in the Tests / Test Go Code job, timing out at ~61s waiting for a forwarded notifications/progress that never arrives.
Observed on commit 7f3d1b27 (PR #6051). That PR contains no production code — only _test.go files and one architecture doc — and does not touch either hop's notification streams, so it cannot be the cause. It surfaced under -race load in CI.
This is a unit-test-job flake, not one of the e2e/operator shard flakes we already know about (the group-rm one in #6040, or the container-runtime suites), so it is worth tracking separately.
Suspected mechanism
The code already documents the race: drainServerToClientNotifications uses a best-effort ping barrier to wait for in-flight server-to-client notifications before the test asserts on them. A ping round-trip is not a guarantee that a previously-sent notification has been delivered and dispatched — it only proves the connection is live and that the ping itself was processed. Under load, particularly with -race slowing every goroutine handoff, a progress notification can still be in flight when the barrier returns and the assertion runs.
Same lineage as the earlier lost-notification work tracked in #5962.
Why the durable fix is deliberate work rather than a retry
The honest options, roughly in order of preference:
- Strengthen the drain barrier so it waits on the actual notification pipeline rather than inferring liveness from a ping — e.g. an explicit sequence/ack the forwarder increments per delivered notification, which the barrier can wait on. This is production code in the forwarding path and wants its own PR and review.
- Poll the assertion with a deadline instead of draining once and asserting. Cheaper, and it converts a hang into a bounded wait, but it weakens the test into "eventually" semantics for something that should be deterministic.
- Raise the timeout. Not recommended — it hides whatever is actually slow and this already sits at 60s.
Option 1 is the real fix. Note that #6051 pins this test to the Legacy revision (progress has no deliverable channel for Modern clients — see #6058), so the flake affects only the Legacy forwarding path, which is the path that has to keep working.
What would help triage
The failure message on timeout does not currently say how many notifications were drained before giving up, or whether the backend had emitted the progress notification at all. A count in the failure output would distinguish "the backend never sent it" from "we asserted before it landed" — worth adding whichever fix is chosen.
Refs #6051, #5962.
Symptom
TestForwarding_Progress_RealBackend(pkg/vmcp/server/forwarding_realbackend_integration_test.go) intermittently fails in theTests / Test Go Codejob, timing out at ~61s waiting for a forwardednotifications/progressthat never arrives.Observed on commit
7f3d1b27(PR #6051). That PR contains no production code — only_test.gofiles and one architecture doc — and does not touch either hop's notification streams, so it cannot be the cause. It surfaced under-raceload in CI.This is a unit-test-job flake, not one of the e2e/operator shard flakes we already know about (the group-rm one in #6040, or the container-runtime suites), so it is worth tracking separately.
Suspected mechanism
The code already documents the race:
drainServerToClientNotificationsuses a best-effort ping barrier to wait for in-flight server-to-client notifications before the test asserts on them. A ping round-trip is not a guarantee that a previously-sent notification has been delivered and dispatched — it only proves the connection is live and that the ping itself was processed. Under load, particularly with-raceslowing every goroutine handoff, a progress notification can still be in flight when the barrier returns and the assertion runs.Same lineage as the earlier lost-notification work tracked in #5962.
Why the durable fix is deliberate work rather than a retry
The honest options, roughly in order of preference:
Option 1 is the real fix. Note that #6051 pins this test to the Legacy revision (progress has no deliverable channel for Modern clients — see #6058), so the flake affects only the Legacy forwarding path, which is the path that has to keep working.
What would help triage
The failure message on timeout does not currently say how many notifications were drained before giving up, or whether the backend had emitted the progress notification at all. A count in the failure output would distinguish "the backend never sent it" from "we asserted before it landed" — worth adding whichever fix is chosen.
Refs #6051, #5962.