Skip to content

Commit 069ba08

Browse files
authored
Detect CRASHED Actors during Checkpoint and Restore (#353)
Related to #292 and #119 Changes: 1. ResumeActor: 1. During `ResumeActor` we'll put the actor in CRASHED state if the Resume failed due to any errors related to the snapshot itself, which includes: 1. Missing or corrupted snapshot when downloading from GCS 2. Runsc command fails. 2. All other types of errors will not put the Actor in CRASHED state. 2. SuspendActor or PauseActor 1. During `SuspendActor` or `PauseActor` we'll put the actor in CRASHED state if we cannot produce a valid snapshot, which includes: 1. cannot connect to Ateom after retries, 2. runsc command fails. 3. cannot upload the snapshot to GCS (during suspend) ; or cannot save snapshot locally (during pause). 2. All other types of errors will not put the Actor in CRASHED state.
1 parent 59d89fe commit 069ba08

26 files changed

Lines changed: 1280 additions & 181 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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+
"fmt"
20+
"log/slog"
21+
22+
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
23+
"github.com/agent-substrate/substrate/internal/ateerrors"
24+
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
25+
"google.golang.org/grpc/codes"
26+
"google.golang.org/grpc/status"
27+
)
28+
29+
// maybeCrashActor inspects err returned by an atelet RPC, it crashes
30+
// the actor if the err carries the actorCrashed=true metadata directive.
31+
func maybeCrashActor(ctx context.Context, st store.Interface, atespace, actorID string, err error, wrapMsg string) error {
32+
if err == nil {
33+
return nil
34+
}
35+
36+
if ateerrors.ActorCrashRequested(err) {
37+
slog.ErrorContext(ctx, "Setting Actor to crashed due to error", slog.Any("error", err))
38+
if cerr := crashActor(ctx, st, atespace, actorID); cerr != nil {
39+
slog.ErrorContext(ctx, "Failed to crash actor", slog.Any("cerr", cerr))
40+
return cerr
41+
}
42+
return status.Errorf(codes.DataLoss, "actor %s crashed", actorID)
43+
}
44+
return fmt.Errorf("%s: %w", wrapMsg, err)
45+
}
46+
47+
// crashActor moves the actor to CRASHED state.
48+
func crashActor(ctx context.Context, st store.Interface, atespace, actorID string) error {
49+
actor, err := st.GetActor(ctx, atespace, actorID)
50+
if err != nil {
51+
return fmt.Errorf("while loading actor to crash: %w", err)
52+
}
53+
actor.Status = ateapipb.Actor_STATUS_CRASHED
54+
// TODO(zoezhao):
55+
// 1. If the Actor crashed because the worker is unhealthy,
56+
// free the worker and mark it as unhealthy(or delete it)
57+
// to prevent other actors from being scheduled on it.
58+
// 2. If the Actor crashed while resuming from a Paused state,
59+
// we must preserve the Actor's assigned node VM in order
60+
// to support `ate actor dump` command.
61+
// (https://github.com/agent-substrate/substrate/issues/119)
62+
if err := st.UpdateActor(ctx, actor, actor.GetVersion()); err != nil {
63+
return fmt.Errorf("while marking actor crashed: %w", err)
64+
}
65+
66+
return nil
67+
}
Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
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+
"strings"
21+
"testing"
22+
23+
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
24+
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store/storetest"
25+
"github.com/agent-substrate/substrate/internal/ateerrors"
26+
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
27+
"google.golang.org/grpc/codes"
28+
"google.golang.org/grpc/status"
29+
)
30+
31+
// seedActor stores a running actor with all worker-binding fields populated, so
32+
// tests can assert they are cleared when the actor crashes.
33+
func seedActor(t *testing.T, ctx context.Context, st store.Interface, atespace, id string) {
34+
t.Helper()
35+
if err := st.CreateActor(ctx, &ateapipb.Actor{
36+
ActorId: id,
37+
Atespace: atespace,
38+
Status: ateapipb.Actor_STATUS_RUNNING,
39+
AteomPodNamespace: "ns",
40+
AteomPodName: "pod",
41+
AteomPodIp: "1.2.3.4",
42+
AteomPodUid: "uid",
43+
WorkerPoolName: "pool",
44+
}); err != nil {
45+
t.Fatalf("seed actor: %v", err)
46+
}
47+
}
48+
49+
// assertCrashed reloads the actor and verifies it is CRASHED.
50+
func assertCrashed(t *testing.T, ctx context.Context, st store.Interface, atespace, id string) {
51+
t.Helper()
52+
got, err := st.GetActor(ctx, atespace, id)
53+
if err != nil {
54+
t.Fatalf("GetActor(%q, %q) = %v, want nil", atespace, id, err)
55+
}
56+
if got.GetStatus() != ateapipb.Actor_STATUS_CRASHED {
57+
t.Errorf("status = %v, want %v", got.GetStatus(), ateapipb.Actor_STATUS_CRASHED)
58+
}
59+
}
60+
61+
func TestCrashActor(t *testing.T) {
62+
const (
63+
atespace = "team-a"
64+
actorID = "actor-1"
65+
)
66+
67+
tests := []struct {
68+
name string
69+
seed bool
70+
// check inspects the returned error; nil-safe.
71+
check func(t *testing.T, ctx context.Context, st store.Interface, err error)
72+
}{
73+
{
74+
name: "crashes running actor",
75+
seed: true,
76+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
77+
if err != nil {
78+
t.Fatalf("crashActor() = %v, want nil", err)
79+
}
80+
assertCrashed(t, ctx, st, atespace, actorID)
81+
},
82+
},
83+
{
84+
name: "actor not found",
85+
seed: false,
86+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
87+
if err == nil {
88+
t.Fatal("crashActor() = nil, want error")
89+
}
90+
if !errors.Is(err, store.ErrNotFound) {
91+
t.Errorf("crashActor() error = %v, want errors.Is(store.ErrNotFound)", err)
92+
}
93+
if !strings.Contains(err.Error(), "while loading actor to crash") {
94+
t.Errorf("crashActor() error = %q, want it to contain %q", err, "while loading actor to crash")
95+
}
96+
},
97+
},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
ctx := context.Background()
103+
st, cleanup := storetest.SetupTestStore(t)
104+
defer cleanup()
105+
106+
if tt.seed {
107+
seedActor(t, ctx, st, atespace, actorID)
108+
}
109+
110+
err := crashActor(ctx, st, atespace, actorID)
111+
tt.check(t, ctx, st, err)
112+
})
113+
}
114+
}
115+
116+
func TestMaybeCrashActor(t *testing.T) {
117+
const (
118+
atespace = "team-a"
119+
actorID = "actor-1"
120+
wrapMsg = "calling atelet"
121+
)
122+
123+
crashErr := ateerrors.NewGRPCError(context.Background(), codes.NotFound, ateerrors.ReasonTerminalFileSystemError, ateerrors.ActorCrashedMetadata(), errors.New("boom"))
124+
// A structured error carrying a reason but no actorCrashed directive must be
125+
// wrapped, not crash the actor.
126+
noCrashErr := ateerrors.NewGRPCError(context.Background(), codes.NotFound, ateerrors.ReasonFailedGetExternalObject, nil, errors.New("infra"))
127+
plainErr := errors.New("transient")
128+
129+
tests := []struct {
130+
name string
131+
seed bool
132+
err error
133+
// check inspects the returned error and store state.
134+
check func(t *testing.T, ctx context.Context, st store.Interface, err error)
135+
}{
136+
{
137+
name: "nil error returns nil",
138+
seed: false,
139+
err: nil,
140+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
141+
if err != nil {
142+
t.Fatalf("maybeCrashActor() = %v, want nil", err)
143+
}
144+
},
145+
},
146+
{
147+
name: "crash reason crashes actor",
148+
seed: true,
149+
err: crashErr,
150+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
151+
if err == nil {
152+
t.Fatal("maybeCrashActor() = nil, want error")
153+
}
154+
if got := status.Code(err); got != codes.DataLoss {
155+
t.Errorf("status code = %v, want %v", got, codes.DataLoss)
156+
}
157+
assertCrashed(t, ctx, st, atespace, actorID)
158+
},
159+
},
160+
{
161+
name: "crash reason but actor missing returns load error",
162+
seed: false,
163+
err: crashErr,
164+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
165+
if err == nil {
166+
t.Fatal("maybeCrashActor() = nil, want error")
167+
}
168+
if got := status.Code(err); got == codes.DataLoss {
169+
t.Errorf("status code = %v, want it not to be DataLoss", got)
170+
}
171+
if !errors.Is(err, store.ErrNotFound) {
172+
t.Errorf("maybeCrashActor() error = %v, want errors.Is(store.ErrNotFound)", err)
173+
}
174+
},
175+
},
176+
{
177+
name: "status error without crash directive is wrapped",
178+
seed: true,
179+
err: noCrashErr,
180+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
181+
if err == nil {
182+
t.Fatal("maybeCrashActor() = nil, want error")
183+
}
184+
if !errors.Is(err, noCrashErr) {
185+
t.Errorf("maybeCrashActor() error = %v, want errors.Is(noCrashErr)", err)
186+
}
187+
if !strings.HasPrefix(err.Error(), wrapMsg) {
188+
t.Errorf("maybeCrashActor() error = %q, want prefix %q", err, wrapMsg)
189+
}
190+
// The actor must not have been crashed.
191+
got, gerr := st.GetActor(ctx, atespace, actorID)
192+
if gerr != nil {
193+
t.Fatalf("GetActor() = %v, want nil", gerr)
194+
}
195+
if got.GetStatus() == ateapipb.Actor_STATUS_CRASHED {
196+
t.Errorf("status = CRASHED, want it unchanged")
197+
}
198+
},
199+
},
200+
{
201+
name: "non-crash error is wrapped",
202+
seed: true,
203+
err: plainErr,
204+
check: func(t *testing.T, ctx context.Context, st store.Interface, err error) {
205+
if err == nil {
206+
t.Fatal("maybeCrashActor() = nil, want error")
207+
}
208+
if !errors.Is(err, plainErr) {
209+
t.Errorf("maybeCrashActor() error = %v, want errors.Is(plainErr)", err)
210+
}
211+
if !strings.HasPrefix(err.Error(), wrapMsg) {
212+
t.Errorf("maybeCrashActor() error = %q, want prefix %q", err, wrapMsg)
213+
}
214+
// The actor must not have been crashed.
215+
got, gerr := st.GetActor(ctx, atespace, actorID)
216+
if gerr != nil {
217+
t.Fatalf("GetActor() = %v, want nil", gerr)
218+
}
219+
if got.GetStatus() == ateapipb.Actor_STATUS_CRASHED {
220+
t.Errorf("status = CRASHED, want it unchanged")
221+
}
222+
},
223+
},
224+
}
225+
226+
for _, tt := range tests {
227+
t.Run(tt.name, func(t *testing.T) {
228+
ctx := context.Background()
229+
st, cleanup := storetest.SetupTestStore(t)
230+
defer cleanup()
231+
232+
if tt.seed {
233+
seedActor(t, ctx, st, atespace, actorID)
234+
}
235+
236+
err := maybeCrashActor(ctx, st, atespace, actorID, tt.err, wrapMsg)
237+
tt.check(t, ctx, st, err)
238+
})
239+
}
240+
}

cmd/ateapi/internal/controlapi/functional_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ type testContext struct {
237237
k8sClient kubernetes.Interface
238238
substrateClient versioned.Interface
239239
persistence *ateredis.Persistence
240+
workerCache *workercache.Cache
240241
fakeAtelet *FakeAteletServer
241242
cleanup func()
242243
actorTemplateLister listersv1alpha1.ActorTemplateLister
@@ -369,6 +370,7 @@ func setupTest(t *testing.T, ns string) *testContext {
369370
k8sClient: k8sClient,
370371
substrateClient: substrateClient,
371372
persistence: persistence,
373+
workerCache: wc,
372374
fakeAtelet: fakeAtelet,
373375
cleanup: cleanup,
374376
actorTemplateLister: actorTemplateLister,
@@ -617,6 +619,23 @@ func createWorkerPod(t *testing.T, tc *testContext, ns string, name string, node
617619
if err != nil {
618620
t.Fatalf("failed to wait for worker to be registered: %v", err)
619621
}
622+
623+
// Wait for the worker to appear in worker cache.
624+
err = wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) {
625+
workers, err := tc.workerCache.Workers()
626+
if err != nil {
627+
return false, nil // Cache not ready yet; retry.
628+
}
629+
for _, w := range workers {
630+
if w.GetWorkerNamespace() == ns && w.GetWorkerPod() == name {
631+
return true, nil
632+
}
633+
}
634+
return false, nil
635+
})
636+
if err != nil {
637+
t.Fatalf("failed to wait for worker to appear in worker cache: %v", err)
638+
}
620639
}
621640

622641
func deleteWorkerPod(t *testing.T, tc *testContext, ns string, name string) {
@@ -642,6 +661,22 @@ func deleteWorkerPod(t *testing.T, tc *testContext, ns string, name string) {
642661
if err != nil {
643662
t.Fatalf("failed to wait for worker to be removed: %v", err)
644663
}
664+
665+
err = wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) {
666+
workers, err := tc.workerCache.Workers()
667+
if err != nil {
668+
return false, nil // Cache not ready yet; retry.
669+
}
670+
for _, w := range workers {
671+
if w.GetWorkerNamespace() == ns && w.GetWorkerPod() == name {
672+
return false, nil // Still there
673+
}
674+
}
675+
return true, nil
676+
})
677+
if err != nil {
678+
t.Fatalf("failed to wait for worker to be removed from worker cache: %v", err)
679+
}
645680
}
646681

647682
// TestCreateActor_Success tests the happy path for creating an actor.

cmd/ateapi/internal/controlapi/syncer.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,9 @@ func (s *WorkerPoolSyncer) releaseActorOnDeadWorker(ctx context.Context, namespa
193193
if actor.GetAteomPodNamespace() != namespace || actor.GetAteomPodName() != podName {
194194
return nil
195195
}
196-
actor.Status = ateapipb.Actor_STATUS_SUSPENDED
196+
if actor.Status != ateapipb.Actor_STATUS_CRASHED {
197+
actor.Status = ateapipb.Actor_STATUS_SUSPENDED
198+
}
197199
actor.AteomPodNamespace = ""
198200
actor.AteomPodName = ""
199201
actor.AteomPodIp = ""

0 commit comments

Comments
 (0)