Skip to content

Commit f93cfcf

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. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
1 parent e2d9cc4 commit f93cfcf

10 files changed

Lines changed: 163 additions & 20 deletions

File tree

charts/substrate-crds/templates/ate.dev_actortemplates.yaml

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,74 @@ spec:
226226
type: object
227227
maxItems: 10
228228
type: array
229+
resources:
230+
description: |-
231+
Resources declares the compute resources for each actor of this template.
232+
Unlike a pod, an actor is sized by its Limits: the sandbox is built to the
233+
CPU/memory limits (cgroup caps, and for the micro-VM the VM's vCPU count and
234+
memory), the scheduler only places the actor on a worker whose capacity is
235+
>= these limits, and the limits are supplied to the sandbox over the actor
236+
RPCs. Because the size is baked into snapshots, it is part of the immutable
237+
spec. Requests and claims are not supported (actors are sized by limits only).
238+
A zero or absent limit leaves the sandbox at the runtime default (unlimited
239+
for gVisor, the kata config for the micro-VM).
240+
properties:
241+
claims:
242+
description: |-
243+
Claims lists the names of resources, defined in spec.resourceClaims,
244+
that are used by this container.
245+
246+
This field depends on the
247+
DynamicResourceAllocation feature gate.
248+
249+
This field is immutable. It can only be set for containers.
250+
items:
251+
description: ResourceClaim references one entry in PodSpec.ResourceClaims.
252+
properties:
253+
name:
254+
description: |-
255+
Name must match the name of one entry in pod.spec.resourceClaims of
256+
the Pod where this field is used. It makes that resource available
257+
inside a container.
258+
type: string
259+
request:
260+
description: |-
261+
Request is the name chosen for a request in the referenced claim.
262+
If empty, everything from the claim is made available, otherwise
263+
only the result of this request.
264+
type: string
265+
required:
266+
- name
267+
type: object
268+
type: array
269+
x-kubernetes-list-map-keys:
270+
- name
271+
x-kubernetes-list-type: map
272+
limits:
273+
additionalProperties:
274+
anyOf:
275+
- type: integer
276+
- type: string
277+
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
278+
x-kubernetes-int-or-string: true
279+
description: |-
280+
Limits describes the maximum amount of compute resources allowed.
281+
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
282+
type: object
283+
requests:
284+
additionalProperties:
285+
anyOf:
286+
- type: integer
287+
- type: string
288+
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
289+
x-kubernetes-int-or-string: true
290+
description: |-
291+
Requests describes the minimum amount of compute resources required.
292+
If Requests is omitted for a container, it defaults to Limits if that is explicitly specified,
293+
otherwise to an implementation-defined value. Requests cannot exceed Limits.
294+
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
295+
type: object
296+
type: object
229297
sandboxClass:
230298
default: gvisor
231299
description: |-
@@ -423,6 +491,17 @@ spec:
423491
rule: '(has(self.sandboxClass) && self.sandboxClass == ''microvm'')
424492
|| !has(self.snapshotsConfig.onResume) || (has(self.snapshotsConfig.onResume.fromData)
425493
? self.snapshotsConfig.onResume.fromData : ''ColdBoot'') != ''Golden'''
494+
- message: spec.resources.requests is not supported; actors are sized
495+
by spec.resources.limits only
496+
rule: '!has(self.resources) || !has(self.resources.requests)'
497+
- message: spec.resources.claims is not supported
498+
rule: '!has(self.resources) || !has(self.resources.claims)'
499+
- message: For sandboxClass 'microvm', spec.resources.limits.memory must
500+
be at least 256Mi (128Mi VMM reserve + 128Mi guest minimum); below
501+
this the VM cannot boot
502+
rule: '!has(self.sandboxClass) || self.sandboxClass != ''microvm'' ||
503+
!has(self.resources) || !has(self.resources.limits) || !(''memory''
504+
in self.resources.limits) || !quantity(self.resources.limits[''memory'']).isLessThan(quantity(''256Mi''))'
426505
status:
427506
description: status is the observed state of ActorTemplate
428507
properties:

cmd/ateapi/internal/actoridentity/actoridentity.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/agent-substrate/substrate/cmd/ateapi/internal/actoridjwt"
3131
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
3232
"github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache"
33-
"github.com/agent-substrate/substrate/internal/k8sjwt"
3433
"github.com/agent-substrate/substrate/internal/localca"
3534
"github.com/agent-substrate/substrate/internal/localjwtauthority"
3635
"github.com/agent-substrate/substrate/internal/principal"

cmd/ateapi/internal/controlapi/dialer_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"testing"
2828
"time"
2929

30+
"github.com/agent-substrate/substrate/internal/installdefaults"
3031
"github.com/agent-substrate/substrate/internal/substratex509"
3132
"github.com/spiffe/go-spiffe/v2/bundle/x509bundle"
3233
"github.com/spiffe/go-spiffe/v2/spiffeid"
@@ -214,7 +215,7 @@ func TestDialForWorkerTarget(t *testing.T) {
214215
Spec: corev1.PodSpec{NodeName: "node-1"},
215216
}
216217
ateletPod := &corev1.Pod{
217-
ObjectMeta: metav1.ObjectMeta{Namespace: ateletNamespace, Name: "atelet-abc", UID: "atelet-uid"},
218+
ObjectMeta: metav1.ObjectMeta{Namespace: installdefaults.SystemNamespace, Name: "atelet-abc", UID: "atelet-uid"},
218219
Spec: corev1.PodSpec{NodeName: "node-1"},
219220
Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: tc.ateletIP}}},
220221
}
@@ -241,7 +242,7 @@ func TestDialForWorkerErrors(t *testing.T) {
241242

242243
t.Run("unknown worker pod", func(t *testing.T) {
243244
ateletPod := &corev1.Pod{
244-
ObjectMeta: metav1.ObjectMeta{Namespace: ateletNamespace, Name: "atelet-abc", UID: "atelet-uid"},
245+
ObjectMeta: metav1.ObjectMeta{Namespace: installdefaults.SystemNamespace, Name: "atelet-abc", UID: "atelet-uid"},
245246
Spec: corev1.PodSpec{NodeName: "node-1"},
246247
Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: "10.244.1.7"}}},
247248
}
@@ -253,7 +254,7 @@ func TestDialForWorkerErrors(t *testing.T) {
253254

254255
t.Run("atelet without assigned IPs", func(t *testing.T) {
255256
ateletPod := &corev1.Pod{
256-
ObjectMeta: metav1.ObjectMeta{Namespace: ateletNamespace, Name: "atelet-abc", UID: "atelet-uid"},
257+
ObjectMeta: metav1.ObjectMeta{Namespace: installdefaults.SystemNamespace, Name: "atelet-abc", UID: "atelet-uid"},
257258
Spec: corev1.PodSpec{NodeName: "node-1"},
258259
}
259260
d := newDialerForPods(t, workerPod, ateletPod)

cmd/ateapi/internal/controlapi/functionaltest/common_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,8 @@ const (
6161
testAtespace = "test-atespace"
6262
testActorID = "id1"
6363

64-
// ateletNamespace and byNode mirror the unexported constants controlapi's
65-
// atelet informer is built with.
66-
ateletNamespace = "ate-system"
67-
byNode = "by-node"
64+
// byNode mirrors the unexported index name controlapi's atelet informer uses.
65+
byNode = "by-node"
6866
)
6967

7068
var (
@@ -187,7 +185,7 @@ func setupTestWithVolumePlugins(t *testing.T, ns string, plugins map[string]volu
187185
mockDriverName: mockPlugin,
188186
}
189187
}
190-
service := controlapi.NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, scLister, dialer, instruments, "", volPlugins)
188+
service := controlapi.NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, scLister, dialer, instruments, "", 30*time.Second, volPlugins)
191189

192190
// 5. Start REAL gRPC Server for ATE API
193191
grpcServer := grpc.NewServer(grpc.UnaryInterceptor(ateinterceptors.ServerUnaryInterceptor))
@@ -594,15 +592,15 @@ func createAteletPod(kc kubernetes.Interface, name, nodeName string) error {
594592
pod := &corev1.Pod{
595593
ObjectMeta: metav1.ObjectMeta{
596594
Name: name,
597-
Namespace: ateletNamespace,
595+
Namespace: installdefaults.SystemNamespace,
598596
Labels: map[string]string{"app": "atelet"},
599597
},
600598
Spec: corev1.PodSpec{
601599
NodeName: nodeName,
602600
Containers: []corev1.Container{{Name: "main", Image: "nginx"}},
603601
},
604602
}
605-
created, err := kc.CoreV1().Pods(ateletNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
603+
created, err := kc.CoreV1().Pods(installdefaults.SystemNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
606604
if apierrors.IsAlreadyExists(err) {
607605
return nil
608606
}
@@ -611,7 +609,7 @@ func createAteletPod(kc kubernetes.Interface, name, nodeName string) error {
611609
}
612610
created.Status.PodIPs = []corev1.PodIP{{IP: "127.0.0.1"}}
613611
created.Status.Phase = corev1.PodRunning
614-
if _, err := kc.CoreV1().Pods(ateletNamespace).UpdateStatus(context.Background(), created, metav1.UpdateOptions{}); err != nil {
612+
if _, err := kc.CoreV1().Pods(installdefaults.SystemNamespace).UpdateStatus(context.Background(), created, metav1.UpdateOptions{}); err != nil {
615613
return fmt.Errorf("updating atelet pod %s status: %w", name, err)
616614
}
617615
return nil
@@ -628,7 +626,7 @@ func setupAteletOnNode(t *testing.T, tc *testContext, name, nodeName string) {
628626
t.Fatalf("%v", err)
629627
}
630628
t.Cleanup(func() {
631-
_ = tc.k8sClient.CoreV1().Pods(ateletNamespace).Delete(context.Background(), name, metav1.DeleteOptions{
629+
_ = tc.k8sClient.CoreV1().Pods(installdefaults.SystemNamespace).Delete(context.Background(), name, metav1.DeleteOptions{
632630
GracePeriodSeconds: ptr.To[int64](0),
633631
})
634632
})

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"
@@ -51,7 +52,8 @@ type VolumePluginRegistry interface {
5152
GetPlugin(ctx context.Context, name string) (volume.VolumePluginControlPlane, error)
5253
}
5354

54-
// NewService creates a service. instruments may be nil; the record helpers no-op.
55+
// NewService creates a service. actorWorkflowDeadline bounds how long a single
56+
// Resume/Suspend workflow can run end-to-end. instruments may be nil.
5557
func NewService(
5658
persistence store.Interface,
5759
workerCache *workercache.Cache,
@@ -63,6 +65,7 @@ func NewService(
6365
dialer *AteletDialer,
6466
instruments *Instruments,
6567
egressGatewayAddress string,
68+
actorWorkflowDeadline time.Duration,
6669
volumePlugins map[string]volume.VolumePluginControlPlane,
6770
) *Service {
6871
s := &Service{
@@ -76,7 +79,7 @@ func NewService(
7679
instruments: instruments,
7780
volumePlugins: volumePlugins,
7881
}
79-
s.actorWorkflow = NewActorWorkflow(persistence, workerCache, dialer, actorTemplateLister, workerPoolLister, sandboxConfigLister, storageClassLister, instruments, egressGatewayAddress, s)
82+
s.actorWorkflow = NewActorWorkflow(persistence, workerCache, dialer, actorTemplateLister, workerPoolLister, sandboxConfigLister, storageClassLister, instruments, egressGatewayAddress, s, actorWorkflowDeadline)
8083
return s
8184
}
8285

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"
@@ -79,9 +80,12 @@ type ActorWorkflow struct {
7980
instruments *Instruments
8081
egressGatewayAddress string
8182
pluginRegistry VolumePluginRegistry
83+
// workflowDeadline is the maximum duration of a single actor workflow.
84+
workflowDeadline time.Duration
8285
}
8386

84-
// NewActorWorkflow creates a new ActorWorkflow. instruments may be nil.
87+
// NewActorWorkflow creates a new ActorWorkflow. workflowDeadline bounds how
88+
// long a single Resume/Suspend can run end-to-end; instruments may be nil.
8589
func NewActorWorkflow(
8690
store actorWorkflowStore,
8791
workerCache *workercache.Cache,
@@ -93,6 +97,7 @@ func NewActorWorkflow(
9397
instruments *Instruments,
9498
egressGatewayAddress string,
9599
pluginRegistry VolumePluginRegistry,
100+
workflowDeadline time.Duration,
96101
) *ActorWorkflow {
97102
return &ActorWorkflow{
98103
store: store,
@@ -106,6 +111,7 @@ func NewActorWorkflow(
106111
instruments: instruments,
107112
egressGatewayAddress: egressGatewayAddress,
108113
pluginRegistry: pluginRegistry,
114+
workflowDeadline: workflowDeadline,
109115
}
110116
}
111117

@@ -124,14 +130,17 @@ type actorWorkflowStore interface {
124130

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

128-
lock, err := w.store.AcquireLock(ctx, lockKey)
135+
lock, err := w.store.AcquireLock(workflowCtx, lockKey)
129136
if err != nil {
137+
cancel()
130138
if errors.Is(err, store.ErrLockConflict) {
131139
return nil, nil, status.Error(grpcCodes.Aborted, "another operation is in progress for this actor")
132140
}
133141
return nil, nil, fmt.Errorf("while acquiring lock: %w", err)
134142
}
135143

144+
context.AfterFunc(lock.Context(), cancel)
136145
return lock.Context(), lock, nil
137146
}
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"
@@ -695,7 +696,7 @@ func TestSuspendActor_PausedWithoutLocalSnapshotCrashes(t *testing.T) {
695696
}); err != nil {
696697
t.Fatalf("add template to indexer: %v", err)
697698
}
698-
w := NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil)
699+
w := NewActorWorkflow(st, nil, nil, listersv1alpha1.NewActorTemplateLister(indexer), nil, nil, nil, nil, "", nil, time.Minute)
699700

700701
seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", ateapipb.ActorState_ACTOR_STATE_PAUSED)
701702

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 state, bound to the given

cmd/ateapi/main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ var (
8888
drainDelay = pflag.Duration("drain-delay", 13*time.Second, "How long to keep accepting new work after SIGTERM, before starting the gRPC drain.")
8989
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.")
9090

91+
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.")
92+
9193
showVersion = pflag.Bool("version", false, "Print version and exit.")
9294
logLevelFlag = pflag.String("log-level", "info", "Minimum log level: debug, info, warn, or error.")
9395
)
@@ -205,7 +207,7 @@ func main() {
205207
dialerOpts = append(dialerOpts, controlapi.WithInsecureCredentials())
206208
}
207209
ateletDialer := controlapi.NewAteletDialer(workerPodInformer.GetIndexer(), ateletPodInformer.GetIndexer(), *ateletClientCredBundle, *podIdentityCACerts, dialerOpts...)
208-
sm := controlapi.NewService(persistence, workerCache, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, storageClassLister, ateletDialer, instruments, *egressGatewayAddress, volPlugins)
210+
sm := controlapi.NewService(persistence, workerCache, actorTemplateLister, workerPoolLister, sandboxConfigLister, csiDriverConfigLister, storageClassLister, ateletDialer, instruments, *egressGatewayAddress, *actorWorkflowDeadline, volPlugins)
209211

210212
actorIdentitySrv := actoridentity.New(actorIdentityJWTIssuer, *actorIDJWTPoolFile, *actorIDCAPoolFile, persistence, workerCache)
211213
debugSrv := debugapi.NewService(persistence)
@@ -327,6 +329,7 @@ func logFlagValues(ctx context.Context) {
327329
slog.Bool("atelet-insecure", *ateletInsecure),
328330
slog.Duration("drain-delay", *drainDelay),
329331
slog.Duration("drain-timeout", *drainTimeout),
332+
slog.Duration("actor-workflow-deadline", *actorWorkflowDeadline),
330333
)
331334
}
332335

0 commit comments

Comments
 (0)