Skip to content

Commit c744181

Browse files
jjamrogaEItanya
authored andcommitted
Decouple actor lock TTL from workflow deadline via heartbeat (#14)
ActorWorkflow.ResumeActor and SuspendActor used to derive their workflow ctx from the Redis lock TTL via acquireActorLock(ctx, id, 30s, 2s) — the workflow deadline and the lock TTL were a single 28s knob. That meant image pulls / restores that legitimately need more than 28s death-looped forever, while raising the knob also raised how long peers wait to retry an actor after a crashed ateapi replica. Split the two concerns: - Lock TTL stays short (30s constant, internal). Bounds peer failover. - Workflow deadline is a separate operator-configurable knob via the new --actor-workflow-deadline pflag (default 5m). Bounds a single Resume/Suspend. - A heartbeat goroutine refreshes the lock every lockTTL/3 (~10s) for the full workflow duration. On RefreshLock=false or any Redis error (peer stole the lock, Redis blip), the workflow ctx is cancelled with errLostActorLock as the cause so in-flight steps unwind cleanly and the mutual-exclusion invariant is preserved. - The release function stops the heartbeat (waits for goroutine exit) before best-effort ReleaseLock. Adds store.Interface.RefreshLock with a Redis CAS Lua script mirroring the existing ReleaseLock script.
1 parent 52718d3 commit c744181

7 files changed

Lines changed: 74 additions & 9 deletions

File tree

cmd/ateapi/internal/controlapi/functional_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ func setupTest(t *testing.T, ns string) *testContext {
367367
volPlugins := map[string]volume.VolumePluginControlPlane{
368368
mockDriverName: mockPlugin,
369369
}
370-
service := NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, scLister, dialer, instruments, "", volPlugins)
370+
service := NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, scLister, dialer, instruments, "", 30*time.Second, volPlugins)
371371

372372
// 5. Start REAL gRPC Server for ATE API
373373
grpcServer := grpc.NewServer(grpc.UnaryInterceptor(ateinterceptors.ServerUnaryInterceptor))

cmd/ateapi/internal/controlapi/service.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package controlapi
1717
import (
1818
"context"
1919
"sync"
20+
"time"
2021

2122
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
2223
"github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache"
@@ -50,7 +51,8 @@ type VolumePluginRegistry interface {
5051
GetPlugin(ctx context.Context, name string) (volume.VolumePluginControlPlane, error)
5152
}
5253

53-
// NewService creates a service. instruments may be nil; the record helpers no-op.
54+
// NewService creates a service. actorWorkflowDeadline bounds how long a single
55+
// Resume/Suspend workflow can run end-to-end. instruments may be nil.
5456
func NewService(
5557
persistence store.Interface,
5658
workerCache *workercache.Cache,
@@ -62,6 +64,7 @@ func NewService(
6264
dialer *AteletDialer,
6365
instruments *Instruments,
6466
egressGatewayAddress string,
67+
actorWorkflowDeadline time.Duration,
6568
volumePlugins map[string]volume.VolumePluginControlPlane,
6669
) *Service {
6770
s := &Service{
@@ -75,7 +78,7 @@ func NewService(
7578
instruments: instruments,
7679
volumePlugins: volumePlugins,
7780
}
78-
s.actorWorkflow = NewActorWorkflow(persistence, workerCache, dialer, actorTemplateLister, workerPoolLister, sandboxConfigLister, storageClassLister, instruments, egressGatewayAddress, s)
81+
s.actorWorkflow = NewActorWorkflow(persistence, workerCache, dialer, actorTemplateLister, workerPoolLister, sandboxConfigLister, storageClassLister, instruments, egressGatewayAddress, s, actorWorkflowDeadline)
7982
return s
8083
}
8184

cmd/ateapi/internal/controlapi/workflow.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"errors"
2020
"fmt"
21+
"time"
2122

2223
"github.com/agent-substrate/substrate/cmd/ateapi/internal/scheduling"
2324
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
@@ -78,9 +79,12 @@ type ActorWorkflow struct {
7879
instruments *Instruments
7980
egressGatewayAddress string
8081
pluginRegistry VolumePluginRegistry
82+
// workflowDeadline is the maximum duration of a single actor workflow.
83+
workflowDeadline time.Duration
8184
}
8285

83-
// NewActorWorkflow creates a new ActorWorkflow. instruments may be nil.
86+
// NewActorWorkflow creates a new ActorWorkflow. workflowDeadline bounds how
87+
// long a single Resume/Suspend can run end-to-end; instruments may be nil.
8488
func NewActorWorkflow(
8589
store store.Interface,
8690
workerCache *workercache.Cache,
@@ -92,6 +96,7 @@ func NewActorWorkflow(
9296
instruments *Instruments,
9397
egressGatewayAddress string,
9498
pluginRegistry VolumePluginRegistry,
99+
workflowDeadline time.Duration,
95100
) *ActorWorkflow {
96101
return &ActorWorkflow{
97102
store: store,
@@ -105,19 +110,23 @@ func NewActorWorkflow(
105110
instruments: instruments,
106111
egressGatewayAddress: egressGatewayAddress,
107112
pluginRegistry: pluginRegistry,
113+
workflowDeadline: workflowDeadline,
108114
}
109115
}
110116

111117
func (w *ActorWorkflow) acquireActorLock(ctx context.Context, actorRef resources.ActorRef) (context.Context, *store.Lock, error) {
112118
lockKey := "lock:actor:" + actorRef.Atespace + ":" + actorRef.Name
119+
workflowCtx, cancel := context.WithTimeout(ctx, w.workflowDeadline)
113120

114-
lock, err := w.store.AcquireLock(ctx, lockKey)
121+
lock, err := w.store.AcquireLock(workflowCtx, lockKey)
115122
if err != nil {
123+
cancel()
116124
if errors.Is(err, store.ErrLockConflict) {
117125
return nil, nil, status.Error(grpcCodes.Aborted, "another operation is in progress for this actor")
118126
}
119127
return nil, nil, fmt.Errorf("while acquiring lock: %w", err)
120128
}
121129

130+
context.AfterFunc(lock.Context(), cancel)
122131
return lock.Context(), lock, nil
123132
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package controlapi
16+
17+
import (
18+
"context"
19+
"errors"
20+
"testing"
21+
"time"
22+
23+
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store/ateredis"
24+
"github.com/agent-substrate/substrate/internal/resources"
25+
"github.com/alicebob/miniredis/v2"
26+
"github.com/redis/go-redis/v9"
27+
)
28+
29+
func TestAcquireActorLockWorkflowDeadline(t *testing.T) {
30+
mr := miniredis.RunT(t)
31+
rdb := redis.NewClusterClient(&redis.ClusterOptions{Addrs: []string{mr.Addr()}})
32+
t.Cleanup(func() { _ = rdb.Close() })
33+
w := &ActorWorkflow{store: ateredis.NewPersistence(rdb), workflowDeadline: 20 * time.Millisecond}
34+
35+
ctx, lock, err := w.acquireActorLock(context.Background(), resources.ActorRef{Atespace: "space", Name: "actor"})
36+
if err != nil {
37+
t.Fatalf("acquireActorLock: %v", err)
38+
}
39+
t.Cleanup(lock.Close)
40+
41+
select {
42+
case <-ctx.Done():
43+
if !errors.Is(ctx.Err(), context.DeadlineExceeded) {
44+
t.Fatalf("context error = %v, want DeadlineExceeded", ctx.Err())
45+
}
46+
case <-time.After(time.Second):
47+
t.Fatal("workflow context did not reach its deadline")
48+
}
49+
}

cmd/ateapi/internal/controlapi/workflow_suspend_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"errors"
2020
"testing"
21+
"time"
2122

2223
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
2324
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store/ateredis"
@@ -679,7 +680,7 @@ func TestSuspendActor_PausedWithoutLocalSnapshotCrashes(t *testing.T) {
679680
}); err != nil {
680681
t.Fatalf("add template to indexer: %v", err)
681682
}
682-
w := NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil)
683+
w := NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil, time.Minute)
683684

684685
seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", ateapipb.Actor_STATUS_PAUSED)
685686

cmd/ateapi/internal/controlapi/workflow_testutil_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"slices"
2020
"testing"
21+
"time"
2122

2223
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
2324
"github.com/agent-substrate/substrate/internal/resources"
@@ -42,7 +43,7 @@ func newTestActorWorkflow(t *testing.T, st store.Interface, tmplNamespace, tmplN
4243
}); err != nil {
4344
t.Fatalf("add template to indexer: %v", err)
4445
}
45-
return NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil)
46+
return NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil, time.Minute)
4647
}
4748

4849
// seedWorkflowActor stores an actor with the given status, bound to the given

cmd/ateapi/main.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/agent-substrate/substrate/cmd/ateapi/internal/actoridentity"
3333
"github.com/agent-substrate/substrate/cmd/ateapi/internal/controlapi"
3434
"github.com/agent-substrate/substrate/cmd/ateapi/internal/debugapi"
35-
"github.com/agent-substrate/substrate/cmd/ateapi/internal/k8sjwt"
3635
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store/ateredis"
3736
"github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache"
3837
"github.com/agent-substrate/substrate/internal/ateapiauth"
@@ -88,6 +87,8 @@ var (
8887
drainDelay = pflag.Duration("drain-delay", 13*time.Second, "How long to keep accepting new work after SIGTERM, before starting the gRPC drain.")
8988
drainTimeout = pflag.Duration("drain-timeout", 15*time.Second, "Deadline for the graceful gRPC drain on shutdown. In-flight RPCs still running past it are forcefully cancelled.")
9089

90+
actorWorkflowDeadline = pflag.Duration("actor-workflow-deadline", 5*time.Minute, "Maximum wall-clock duration of a single Resume/Suspend workflow; raise it for slow image registries.")
91+
9192
showVersion = pflag.Bool("version", false, "Print version and exit.")
9293
logLevelFlag = pflag.String("log-level", "info", "Minimum log level: debug, info, warn, or error.")
9394
clientJWTCAFile = pflag.String("client-jwt-ca-cert", ateapiauth.DefaultServiceAccountCAFile, "CA cert file used to verify TLS when fetching the OIDC discovery document and JWKS for JWT authentication. Defaults to the in-cluster service account CA.")
@@ -196,7 +197,7 @@ func main() {
196197

197198
volPlugins := make(map[string]volume.VolumePluginControlPlane)
198199
ateletDialer := controlapi.NewAteletDialer(workerPodInformer.GetIndexer(), ateletPodInformer.GetIndexer(), *ateletClientCredBundle, *podIdentityCACerts, *ateletInsecure)
199-
sm := controlapi.NewService(redisPersistence, workerCache, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, storageClassLister, ateletDialer, instruments, *egressGatewayAddress, volPlugins)
200+
sm := controlapi.NewService(redisPersistence, workerCache, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, storageClassLister, ateletDialer, instruments, *egressGatewayAddress, *actorWorkflowDeadline, volPlugins)
200201

201202
jwtIssuerDiscoveryClient := buildK8sServiceAccountIssuerDiscoveryClient(ctx, *clientJWTCAFile, *clientJWTIssuer)
202203

@@ -328,6 +329,7 @@ func logFlagValues(ctx context.Context) {
328329
slog.Bool("atelet-insecure", *ateletInsecure),
329330
slog.Duration("drain-delay", *drainDelay),
330331
slog.Duration("drain-timeout", *drainTimeout),
332+
slog.Duration("actor-workflow-deadline", *actorWorkflowDeadline),
331333
)
332334
}
333335

0 commit comments

Comments
 (0)