fix: Heartbeat ReportState for long-running silent jobs (#852)

Fixes #826.

Regressed in f2d54556 (#819, "perf: reduce runner-to-server connection load with adaptive reporting and polling"). That change added an early-return in `ReportState` whenever there was no state change and no pending outputs, so jobs that produce no log output and no step transitions for many minutes (e.g. a Linux kernel build) stop heartbeating. The server eventually marks the task as orphaned and cancels it while the runner is still executing.

The fix tracks the last successful `UpdateTask` time in an atomic and keeps the no-op skip only while the previous report is younger than `stateReportInterval`. The periodic state ticker fires at exactly `stateReportInterval`, so silent jobs now heartbeat each tick; redundant sends from a `stateNotify` firing right after a tick are still suppressed, preserving the perf intent of #819.

Test added: `TestReporter_StateHeartbeat` asserts the skip path within the interval and the heartbeat path after the interval elapses.

---
This PR was written with the help of Claude Opus 4.7

Reviewed-on: https://gitea.com/gitea/runner/pulls/852
Reviewed-by: Nicolas <bircni@icloud.com>
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-by: ChristopherHX <38043+christopherhx@noreply.gitea.com>
Co-authored-by: silverwind <me@silverwind.io>
Co-committed-by: silverwind <me@silverwind.io>
This commit is contained in:
silverwind
2026-04-26 22:49:39 +00:00
committed by silverwind
parent 7c6f1261d4
commit fa5334eb24
2 changed files with 53 additions and 1 deletions

View File

@@ -10,6 +10,7 @@ import (
"regexp"
"strings"
"sync"
"sync/atomic"
"time"
"gitea.com/gitea/act_runner/internal/pkg/client"
@@ -48,6 +49,10 @@ type Reporter struct {
outputs sync.Map
daemon chan struct{}
// Unix-nanos of the last successful UpdateTask. Atomic so the heartbeat
// guard in ReportState reads it without contending stateMu.
lastReportedAtNanos atomic.Int64
// Adaptive batching control
logReportInterval time.Duration
logReportMaxLatency time.Duration
@@ -489,8 +494,12 @@ func (r *Reporter) ReportState(reportResult bool) error {
// Consume stateChanged atomically with the snapshot; restored on error
// below so a concurrent Fire() during UpdateTask isn't silently lost.
// Heartbeat at stateReportInterval even when nothing changed, so the server
// doesn't time out long-running silent jobs as orphaned (#826).
last := r.lastReportedAtNanos.Load()
withinHeartbeatInterval := last != 0 && time.Since(time.Unix(0, last)) < r.stateReportInterval
r.stateMu.Lock()
if !reportResult && !r.stateChanged && len(outputs) == 0 {
if !reportResult && !r.stateChanged && len(outputs) == 0 && withinHeartbeatInterval {
r.stateMu.Unlock()
return nil
}
@@ -517,6 +526,7 @@ func (r *Reporter) ReportState(reportResult bool) error {
return err
}
metrics.ReportStateTotal.WithLabelValues(metrics.LabelResultSuccess).Inc()
r.lastReportedAtNanos.Store(time.Now().UnixNano())
for _, k := range resp.Msg.SentOutputs {
r.outputs.Store(k, struct{}{})

View File

@@ -597,3 +597,45 @@ func TestReporter_StateNotifyFlush(t *testing.T) {
}, 500*time.Millisecond, 10*time.Millisecond,
"step transition should have triggered immediate state flush via stateNotify")
}
// TestReporter_StateHeartbeat verifies that ReportState sends a heartbeat
// UpdateTask once stateReportInterval has elapsed since the last successful
// report, even when nothing has changed. Without this, long-running silent
// jobs (no log output, no step transitions) cause the server to time the
// task out and cancel it (#826).
func TestReporter_StateHeartbeat(t *testing.T) {
var updateTaskCalls atomic.Int64
client := mocks.NewClient(t)
client.On("UpdateTask", mock.Anything, mock.Anything).Return(
func(_ context.Context, _ *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) {
updateTaskCalls.Add(1)
return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
},
)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
taskCtx, err := structpb.NewStruct(map[string]any{})
require.NoError(t, err)
cfg, _ := config.LoadDefault("")
cfg.Runner.StateReportInterval = 50 * time.Millisecond
reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx}, cfg)
reporter.ResetSteps(1)
// First call has no prior report — sends to seed lastReportedAt.
reporter.stateMu.Lock()
reporter.stateChanged = true
reporter.stateMu.Unlock()
require.NoError(t, reporter.ReportState(false))
require.Equal(t, int64(1), updateTaskCalls.Load())
// Second call immediately after with nothing changed — must skip.
require.NoError(t, reporter.ReportState(false))
assert.Equal(t, int64(1), updateTaskCalls.Load(), "no-op ReportState within stateReportInterval must skip")
// After stateReportInterval elapses, a heartbeat must fire even with no changes.
time.Sleep(2 * cfg.Runner.StateReportInterval)
require.NoError(t, reporter.ReportState(false))
assert.Equal(t, int64(2), updateTaskCalls.Load(), "ReportState must heartbeat after stateReportInterval even with no state change")
}