agent-substrate / agent-substrate/substrate

[P2] Partial GCS/S3 upload on checkpoint leaves orphaned objects — bucket grows without bound

Đang mở
#610 0 bình luận 0 reaction 1 người được giao Được @EItanya nhận Xem trên GitHub
area/storage kind/bug prio/P2
Ngôn ngữ chính
Go
Star
1.8k
Fork
316
Merge trung bình
2 ngày 43 phút
Pull request đã merge (30 ngày)
287

Mô tả

**Severity:** P2 (silent storage cost growth; orphans are permanent)
**Component:** Node Layer — `cmd/atelet/main.go`
**Audit ID:** NL-7

---

## Summary

`uploadExternalCheckpoint` launches one goroutine per snapshot file. If any goroutine
fails, already-uploaded files are **not cleaned up** — they remain as orphaned objects
in the bucket. Similarly, if the manifest upload fails after all data files upload
successfully, the snapshot data is in the bucket with no manifest. No `DeleteObject`
call exists anywhere in the codebase. Over time, partial failed checkpoints accumulate
as unreferenced objects consuming storage.

---

## Root Cause

**File:** `cmd/atelet/main.go` lines 458–488

```go
g, gCtx := errgroup.WithContext(ctx)
for _, file := range snapshotFiles {
file := file
g.Go(func() error {
return SendLocalFileToGCSWithZstd(gCtx, client, bucket, file)
// If this goroutine SUCCEEDS, objects are in GCS
})
}
if err := g.Wait(); err != nil {
// One goroutine failed — but successful goroutines already wrote objects
// No cleanup of the already-uploaded objects
return err
}
// Upload manifest
if err := writeManifest(ctx, client, bucket, manifest); err != nil {
// Data files are in GCS, manifest is not — orphaned data
return err
}
```

`grep -rn "DeleteObject\|DeleteObjects\|Objects().Delete" /Users/adityashantanu/repos/substrate/` — zero results.

---

## Steps to Reproduce

1. Configure GCS to accept data file PUTs but reject the manifest PUT (e.g., via
a custom object name ACL or a test proxy that drops requests for `manifest.json`).
2. Trigger a checkpoint:
```bash
kubectl ate suspend actor my-actor -a demo
```
3. Observe: `SuspendActor` returns an error; actor is CRASHED (per ISSUE-002).
4. List GCS objects for the actor:
```bash
gcloud storage ls -r gs://///
# Shows: memory-ranges.zstd, disk.zstd, ... (all data files)
# Missing: manifest.json
```
5. These objects remain in the bucket permanently:
```bash
# After deleting the actor
kubectl ate delete actor my-actor -a demo
gcloud storage ls -r gs://///
# Still shows the orphaned data files
```

---

## Expected Behavior

On partial upload failure, atelet should attempt to delete already-uploaded objects
(best-effort). If the cleanup also fails, the objects should be reported to a "orphan
cleanup" registry or the bucket should have an object lifecycle policy to auto-expire
objects without a companion `manifest.json` after 24 hours.

---

## Suggested Fix

**Short-term:** Add a best-effort cleanup in `uploadExternalCheckpoint` on failure:

```go
if err := g.Wait(); err != nil {
// Best-effort cleanup
for _, uploaded := range successfullyUploaded {
_ = client.DeleteObject(ctx, bucket, uploaded)
}
return err
}
```

**Long-term:** Configure GCS lifecycle rules on the snapshot bucket:
```json
{
"rule": [{
"condition": {"age": 2, "matchesSuffix": [".zstd"]},
"action": {"type": "Delete"}
}]
}
```
This provides a safety net for any orphaned partial uploads.

Also add a periodic bucket audit that finds objects without a companion `manifest.json`
in the same prefix and deletes them (or alerts on them).

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.