Repository navigation
Conversation
e4b7a42 to
3d50d6b
Compare
3d50d6b to
924ff73
Compare
924ff73 to
f19f33b
Compare
6fc6a83 to
4043d3c
Compare
THardy98
left a comment
THardy98
left a comment
There was a problem hiding this comment.
Left a comment - not blocking, but would like to understand the reasoning behind the comment.
| }), DefaultRemoteActivity) | ||
| } | ||
|
|
||
| // A missed heartbeat must fail the load. |
There was a problem hiding this comment.
is this codified anywhere?
i dont immediately see why it should cause a failure - is it scenario-specific?
There was a problem hiding this comment.
This is specific to the optional healthy-heartbeat probe, rather than a general rule for all activities.
The intent is to exercise an activity that stays alive through sustained heartbeating. It has MaximumAttempts: 1 so an activity retry cannot mask a heartbeat timeout. That policy is encoded in healthyHeartbeatActivity and asserted by TestThroughputStressHealthyHeartbeatActivities; the real-worker test verifies that the activity survives longer than its heartbeat timeout.
The comment is too broad, though. A failed individual heartbeat RPC does not necessarily fail the activity: subsequent heartbeats can still arrive before the timeout. An actual heartbeat timeout fails this activity and propagates through the scenario’s existing failure handling.
I will narrow the comment to explain the single-attempt choice: “Keep heartbeat timeouts observable rather than masking them with activity retries.” This is a probe-specific testing choice, not a general requirement that every missed heartbeat must fail a load.
throughput_stress only heartbeats in its retry scenario, where the first attempt deliberately times out. Nothing exercises activities that stay alive by heartbeating, so load runs cover the heartbeat failure path but not sustained successful heartbeats. healthy-heartbeat-activities adds that many remote activities to each internal iteration. Each heartbeats for healthy-heartbeat-duration with a healthy-heartbeat-timeout heartbeat timeout and a single attempt, so a heartbeat timeout fails the run. Off by default. Uses the Go worker's existing heartbeat activity, so no worker change is needed.
4043d3c to
e784836
Compare
What changed
Add opt-in healthy heartbeat activities to each throughput iteration, with configurable count, duration and heartbeat timeout. Each has a single attempt, so a heartbeat timeout fails the workflow and load. Nonpositive and overflowing timing configurations are rejected before execution.
Review: As a single unit in stack order.
Why
The existing retry probes intentionally withhold heartbeats on their first attempt. They cover failure handling but do not exercise activities that must stay alive by heartbeating throughout useful work.
Details worth reviewing
Merge after #518, including its SDK prerequisite. The activity count defaults to zero; the action uses the Go worker's existing heartbeat support. Local heartbeat calls remain subject to SDK transport batching.