fix: make volume deletion asynchronous so a large volume cannot fail its own delete (#1356)

DeleteVolume returned only once every object behind the volume was gone, which
made the call's latency a function of the volume's size. Object removal costs
one round trip per object, so no timeout is both short enough for a control
plane request and long enough for a large volume: on a 100 GiB volume the
provider request overran its 30 s budget at 52 s, the tenant was told the
delete failed, and the teardown that had already started was left with no
record accountable for it.

AWS does not work this way. DeleteVolume's output in the EC2 model is
smithy.api#Unit, VolumeState includes deleting, and the documentation says a
volume can remain in that state for several minutes.

DeleteVolume now validates, decides every AWS-returnable error, records
State=deleting and returns. VolumeReclaimReaper converges the teardown and
removes the record only once it is complete, so an unreachable or saturated
blob node delays reclamation instead of failing a delete the tenant has already
been told succeeded. The reaper is cluster-wide and leader-gated, which is also
what serialises the teardown: three concurrent destructive handlers ran one
volume's objects on prod because a retry had nothing to collapse against.

A volume already recorded as deleting answers a second delete from the record,
which collapses both the tenant's retry and DeleteOnTermination's own delete.

Two ordering fixes behind it. The provider handler now tears down local state
before deleting objects, because the local half is fixed-cost and sat behind the
half that scales, so a timeout left this node's WAL and dirty marker behind for
good. And volume.delete and snapshot.delete are classified as data movement,
since both reference real data and neither answers from state already held.

spinifex/domains/ec2/image/account_termination_current_behaviour_test.go

@@ -111,6 +111,10 @@ func TestCurrentBehaviour_AccountTermination_CreateImageSnapshotDoesNotPinSource
 	imageID := s.createImage(t, "vol-imaged", "ct-image")
 	require.NoError(t, s.deleteVolume("vol-imaged"), "a CreateImage snapshot does not block deleting its source volume")
 
+	// DeleteVolume records the obligation and returns, so the document is gone
+	// once the reclaimer has converged it rather than when the call returns.
+	_, err = s.volumes.NewVolumeReclaimReaper().Sweep(ctx)
+	require.NoError(t, err)
 	_, err = s.metadata.GetVolume(ctx, ctAccountID, "vol-imaged")
 	require.Error(t, err, "the source volume's document is gone")
 

spinifex/domains/ec2/volume/delete_volume_unreadable_test.go

@@ -103,8 +103,10 @@ func (s *racingDeleteStore) DeleteObject(ctx context.Context, input *s3.DeleteOb
 	return s.MemoryObjectStore.DeleteObject(ctx, input)
 }
 
-// A document another delete removed after this one read it is deleted, so the
-// call must succeed rather than answer InternalError for a finished delete.
+// A document another sweep removed after this one read it is deleted, so the
+// reclaimer must count it done rather than fail the sweep for a finished
+// delete. The record is the obligation, so retiring it is the reclaimer's step
+// and this is where a concurrent remover is now observed.
 func TestDeleteVolume_ConcurrentlyDeletedDocumentIsSuccess(t *testing.T) {
 	memory := objectstore.NewMemoryObjectStore()
 	svc := newTestVolumeServiceWithStore("ap-southeast-2a", memory)
@@ -115,15 +117,19 @@ func TestDeleteVolume_ConcurrentlyDeletedDocumentIsSuccess(t *testing.T) {
 		VolumeID: volumeID, CapacityGiB: 10, State: "available",
 	})
 
-	racing := &racingDeleteStore{MemoryObjectStore: memory}
-	svc.store = racing
-	svc.metadata = ebsmetadata.NewStore(racing, "test-bucket")
-
 	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{
 		VolumeId: aws.String(volumeID),
 	}, testVolAccountID)
 	require.NoError(t, err)
 
+	racing := &racingDeleteStore{MemoryObjectStore: memory}
+	svc.store = racing
+	svc.metadata = ebsmetadata.NewStore(racing, "test-bucket")
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err, "a document a concurrent sweep removed is a finished delete, not a sweep failure")
+	assert.Equal(t, 1, reclaimed)
+
 	_, getErr := ebsmetadata.NewStore(memory, "test-bucket").GetVolume(context.Background(), testVolAccountID, volumeID)
 	assert.True(t, objectstore.IsNoSuchKeyError(getErr), "the document must be gone, got %v", getErr)
 }

spinifex/domains/ec2/volume/lifecycle_invariants_test.go

@@ -97,6 +97,52 @@ func TestRLC3_VolumeDeleteRequiresDetach(t *testing.T) {
 		"ADR-0003:S2 behaviour; historic PROP-LIFECYCLE-005 §2 mechanism: deleting an attached volume must return VolumeInUse (detach-before-delete, rule #3)")
 }
 
+// TestS2_VolumeDeleteRecordsTheObligationBeforeRemovingTheRecord enforces
+// ADR-0003:S2's delete contract: "Delete first creates a durable obligation and
+// prevents new dependants ... The active record is removed only when all
+// required cleanup is durably complete."
+//
+// A delete that removes the record as part of the request cannot satisfy this,
+// because the cleanup it depends on costs one round trip per object and so has
+// no bound a request can carry. On prod a 100 GiB volume's delete overran its
+// 30 s budget, the tenant was told it failed, and the teardown that had been
+// started was left with no record accountable for it.
+func TestS2_VolumeDeleteRecordsTheObligationBeforeRemovingTheRecord(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
+	svc.snapshotKV = setupTestVolumeKV(t)
+	seedVolume(t, svc, "vol-obligation000", "available", "")
+
+	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String("vol-obligation000")}, testVolAccountID)
+	require.NoError(t, err)
+
+	owed, err := svc.GetVolumeMetadata(testVolAccountID, "vol-obligation000")
+	require.NoErrorf(t, err, "ADR-0003:S2: the active record must survive the request so the obligation stays readable (service_impl.go DeleteVolume: record the state, do not remove the document)")
+	assert.Equalf(t, volumeStateDeleting, owed.State,
+		"ADR-0003:S2: delete must first create a durable obligation (service_impl.go DeleteVolume: write State=%q before returning)", volumeStateDeleting)
+
+	// The obligation is what callers see, so describe must report it rather than
+	// the available state the volume no longer has. The attach refusal reads the
+	// same field (runtime/daemon/daemon_handlers_volume.go: State != "available").
+	described, err := svc.DescribeVolumes(context.Background(), &ec2.DescribeVolumesInput{
+		VolumeIds: []*string{aws.String("vol-obligation000")},
+	}, testVolAccountID)
+	require.NoError(t, err)
+	require.Len(t, described.Volumes, 1)
+	assert.Equalf(t, volumeStateDeleting, aws.StringValue(described.Volumes[0].State),
+		"ADR-0003:S2: a delete must prevent new dependants, and %q is the state every dependant path gates on", volumeStateDeleting)
+
+	// A second delete adds nothing rather than racing the first teardown.
+	_, err = svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String("vol-obligation000")}, testVolAccountID)
+	require.NoErrorf(t, err, "ADR-0003:S2: the obligation is durable, so a repeated delete is answered from it (service_impl.go DeleteVolume: return early on State==%q)", volumeStateDeleting)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	_, err = svc.GetVolumeMetadata(testVolAccountID, "vol-obligation000")
+	require.Errorf(t, err, "ADR-0003:S2: the active record is removed once cleanup is durably complete (volume_reclaim_reaper.go Sweep: delete the document last)")
+}
+
 // TestVolumeTagMirror_WritesOnlyControlPlaneDocument locks the single-writer
 // invariant: tag mutations must never rewrite config.json or route an encrypted
 // volume through the ebs.config keyholder queue.

spinifex/domains/ec2/volume/service_impl.go

@@ -33,6 +33,11 @@ import (
 	"github.com/nats-io/nats.go/jetstream"
 )
 
+// volumeStateDeleting is the state a delete records as its durable obligation.
+// The record carries it until the teardown is complete, which is what makes a
+// delete survive a restart and what prevents a second one repeating the work.
+const volumeStateDeleting = string(ebsprovider.VolumeStateDeleting)
+
 // Ensure VolumeServiceImpl implements VolumeService.
 var _ VolumeService = (*VolumeServiceImpl)(nil)
 
@@ -1078,6 +1083,14 @@ func (s *VolumeServiceImpl) ModifyVolume(ctx context.Context, input *ec2.ModifyV
 // is always a metadata-only clear, never a QMP call. Errors from either step
 // are returned, not swallowed.
 func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error {
+	// Clearing the attachment would overwrite a recorded deletion, so a volume
+	// already owed is left alone: the reclaimer owns it from here, and resetting
+	// it to available would let a second teardown run against the same data.
+	meta, err := s.metadata.GetVolume(ctx, accountID, volumeID)
+	if err == nil && meta.State == volumeStateDeleting {
+		return nil
+	}
+
 	if err := s.UpdateVolumeState(accountID, volumeID, "available", "", ""); err != nil {
 		// No metadata doc means the volume is already gone, per GetVolume's own
 		// contract, so absence here is success, not a teardown failure.
@@ -1086,7 +1099,7 @@ func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeI
 		}
 		return fmt.Errorf("clear attachment before terminate delete: %w", err)
 	}
-	_, err := s.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: &volumeID}, accountID)
+	_, err = s.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: &volumeID}, accountID)
 	return err
 }
 
@@ -1154,8 +1167,17 @@ func (s *VolumeServiceImpl) ForceDetachVolume(ctx context.Context, input *ec2.De
 	}, nil
 }
 
-// DeleteVolume deletes an EBS volume: validates state, asks the EBS provider to
-// destroy the backing data, then removes the control-plane metadata document.
+// DeleteVolume records a durable obligation to destroy an EBS volume and
+// returns, matching AWS: DeleteVolume's output is an empty shape and the
+// documented state machine has the volume "remain in the deleting state for
+// several minutes". It validates, decides every AWS-returnable error, moves the
+// volume to "deleting", and leaves the object teardown to VolumeReclaimReaper.
+//
+// The caller never waits on data. Object deletion costs one round trip per
+// object, so the old synchronous form made this call's latency a function of
+// the volume's size: no timeout can be both short enough for a control-plane
+// request and long enough for a large volume, and the overrun contributed no
+// error a caller could act on.
 func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteVolumeInput, accountID string) (*ec2.DeleteVolumeOutput, error) {
 	if input == nil || input.VolumeId == nil || *input.VolumeId == "" {
 		return nil, errors.New(awserrors.ErrorInvalidParameterValue)
@@ -1182,6 +1204,14 @@ func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteV
 		return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
 	}
 
+	// The obligation is already recorded, so this request has nothing to add.
+	// That makes a tenant's retry and DeleteOnTermination's own delete for the
+	// same volume collapse into one, rather than racing each other's teardown.
+	if meta.State == volumeStateDeleting {
+		slog.InfoContext(ctx, "DeleteVolume: already deleting", "volumeId", volumeID)
+		return &ec2.DeleteVolumeOutput{}, nil
+	}
+
 	// An unattached volume is deletable. State must be "available" OR empty: a
 	// detach/terminate that failed to write back "available" leaves the State
 	// drifted to empty with no attachment, and gating on State=="available"
@@ -1196,19 +1226,16 @@ func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteV
 	if err := s.checkVolumeHasNoSnapshots(ctx, volumeID); err != nil {
 		return nil, err
 	}
-	if err := s.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{
-		Versioned: ebsprovider.NewVersioned(), VolumeID: volumeID, Handle: meta.ProviderHandle,
-	}); err != nil {
-		slog.ErrorContext(ctx, "DeleteVolume: provider deletion failed", "volumeId", volumeID, "err", err)
-		// The terminate reaper retries this every two minutes and logs whatever
-		// it gets back, so a bare code makes a persistent fault unreadable.
-		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "delete volume %s: %v", volumeID, err)
-	}
-	if err := s.metadata.DeleteVolume(ctx, accountID, volumeID); err != nil && !objectstore.IsNoSuchKeyError(err) {
-		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "delete volume metadata %s: %v", volumeID, err)
+
+	// Last write of the request, and the only one that can fail from here: the
+	// record is the obligation, so until this lands nothing is owed and the
+	// caller must not be told otherwise.
+	if err := s.UpdateVolumeState(accountID, volumeID, volumeStateDeleting, "", ""); err != nil {
+		slog.ErrorContext(ctx, "DeleteVolume: failed to record the deletion", "volumeId", volumeID, "err", err)
+		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "mark volume %s deleting: %v", volumeID, err)
 	}
 
-	slog.InfoContext(ctx, "DeleteVolume completed", "volumeId", volumeID)
+	slog.InfoContext(ctx, "DeleteVolume accepted", "volumeId", volumeID)
 
 	return &ec2.DeleteVolumeOutput{}, nil
 }

spinifex/domains/ec2/volume/service_impl_test.go

@@ -326,6 +326,12 @@ func TestCreateVolume_UsesInjectedProvider(t *testing.T) {
 	require.NoError(t, svc.UpdateVolumeState(testVolAccountID, aws.StringValue(vol.VolumeId), "available", "", ""))
 	_, err = svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: vol.VolumeId}, testVolAccountID)
 	require.NoError(t, err)
+	deleting, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, aws.StringValue(vol.VolumeId))
+	require.NoError(t, err, "the record carries the obligation until the teardown is complete")
+	assert.Equal(t, "deleting", deleting.State)
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
 	_, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, aws.StringValue(vol.VolumeId))
 	assert.Error(t, err)
 }
@@ -1764,8 +1770,16 @@ func TestDeleteVolumeOnTerminate_ClearsAttachmentThenDeletes(t *testing.T) {
 	err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID)
 	require.NoError(t, err, "terminate implies detach: a stale attachment must not block the terminate delete")
 
+	deleting, err := svc.GetVolumeMetadata(testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", deleting.State, "the volume must be recorded as deleting, not merely detached")
+	assert.Empty(t, deleting.AttachedInstance)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
 	_, err = svc.GetVolumeMetadata(testVolAccountID, volumeID)
-	require.Error(t, err, "the volume must actually be deleted, not merely detached")
+	require.Error(t, err, "the volume must actually be deleted once the teardown completes")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)
 }
 
@@ -1809,11 +1823,12 @@ func (p *failingDeleteProvider) DeleteVolume(context.Context, ebsprovider.Delete
 	return p.err
 }
 
-// TestDeleteVolume_ProviderFailureCarriesItsCause pins that a provider delete
-// failure keeps its cause on the way out. The terminate reaper retries this
-// every two minutes and logs whatever it is handed, so a bare ServerInternal
-// makes a persistent fault indistinguishable from a transient one.
-func TestDeleteVolume_ProviderFailureCarriesItsCause(t *testing.T) {
+// TestDeleteVolume_ProviderFailureDoesNotFailTheRequest pins the behaviour a
+// 100 GiB volume on prod needed: an unreachable blob node must not turn an
+// accepted delete into an error the tenant retries. The obligation is already
+// recorded, so the request succeeds and the teardown is the reclaimer's to
+// retry until it lands.
+func TestDeleteVolume_ProviderFailureDoesNotFailTheRequest(t *testing.T) {
 	kv := setupTestVolumeKV(t)
 	store := objectstore.NewMemoryObjectStore()
 	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
@@ -1829,12 +1844,74 @@ func TestDeleteVolume_ProviderFailureCarriesItsCause(t *testing.T) {
 	})
 
 	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, testVolAccountID)
-	require.Error(t, err)
+	require.NoError(t, err, "a provider that cannot be reached must not fail a delete the control plane has accepted")
 
-	assert.Equal(t, awserrors.ErrorServerInternal, awserrors.ValidErrorCodeFromError(err),
-		"the code stays ServerInternal so callers classify it unchanged")
-	assert.Contains(t, err.Error(), "dial node 6",
-		"an operator reading the reaper log must learn why the delete keeps failing")
+	meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", meta.State)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err, "a failed teardown is retried, not reported as a sweep failure")
+	assert.Equal(t, 0, reclaimed)
+	meta, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err, "the record survives so the obligation is still readable")
+	assert.Equal(t, "deleting", meta.State)
+
+	// The provider comes back, and the same record is what drives the retry.
+	svc.SetEBSProvider(&failingDeleteProvider{EBSProvider: svc.provider, err: nil})
+	reclaimed, err = svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	_, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	assert.Error(t, err)
+}
+
+// TestDeleteVolume_SecondDeleteIsAcceptedWithoutASecondTeardown pins the dedupe
+// the deleting state buys. A tenant retry and DeleteOnTermination's own delete
+// both arrive for the same volume, and on prod three destructive handlers ran
+// one volume's teardown concurrently because nothing collapsed them.
+func TestDeleteVolume_SecondDeleteIsAcceptedWithoutASecondTeardown(t *testing.T) {
+	kv := setupTestVolumeKV(t)
+	store := objectstore.NewMemoryObjectStore()
+	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
+	svc.snapshotKV = kv
+
+	volumeID := "vol-deleted-twice"
+	createVolumeInStoreWithMeta(t, svc, store, volumeID, ebsmetadata.Volume{
+		VolumeID: volumeID, CapacityGiB: 10, State: "available",
+	})
+	counting := &countingDeleteProvider{EBSProvider: svc.provider}
+	svc.SetEBSProvider(counting)
+
+	for range 3 {
+		_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, testVolAccountID)
+		require.NoError(t, err, "AWS answers a repeated delete with the same empty success")
+	}
+	assert.Equal(t, 0, counting.calls, "no teardown runs in the request path at all")
+
+	// DeleteOnTermination arriving for a volume already owed must not reset it
+	// to available, which would let a second teardown through.
+	require.NoError(t, svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID))
+	meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", meta.State)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	assert.Equal(t, 1, counting.calls, "one recorded obligation is one teardown")
+}
+
+// countingDeleteProvider counts the deletes that reach the provider boundary.
+type countingDeleteProvider struct {
+	ebsprovider.EBSProvider
+
+	calls int
+}
+
+func (p *countingDeleteProvider) DeleteVolume(context.Context, ebsprovider.DeleteVolumeRequest) error {
+	p.calls++
+	return nil
 }
 
 // TestDeleteVolumeOnTerminate_SurfacesDeleteFailure verifies a DeleteVolume

spinifex/domains/ec2/volume/volume_reclaim_reaper.go

@@ -0,0 +1,93 @@
+package volume
+
+import (
+	"context"
+	"log/slog"
+
+	ebsprovider "github.com/mulgadc/spinifex/spinifex/providers/ebs"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
+)
+
+// VolumeReclaimReaper converges the volumes a tenant has already asked to
+// delete. DeleteVolume records the obligation as State="deleting" and returns,
+// so the object teardown runs here: its cost is one round trip per object and
+// therefore a function of the volume, which is not work a caller can be held
+// on. The active record is removed only once the teardown reports complete.
+//
+// Its licence to delete volume data is narrow and is not VolumeLeakReaper's.
+// That reaper may only mark and alarm because it would be deciding, on its own
+// judgement, that a volume is garbage. This one decides nothing: the tenant
+// decided, the decision is recorded on the volume itself, and the sweep only
+// finishes what the API accepted.
+type VolumeReclaimReaper struct {
+	svc *VolumeServiceImpl
+}
+
+var _ vm.Reaper = (*VolumeReclaimReaper)(nil)
+
+// NewVolumeReclaimReaper builds the reclaimer for volumes recorded as deleting.
+func (s *VolumeServiceImpl) NewVolumeReclaimReaper() *VolumeReclaimReaper {
+	return &VolumeReclaimReaper{svc: s}
+}
+
+func (r *VolumeReclaimReaper) Class() string { return "volume-reclaim" }
+
+// Scope is cluster-wide, which is also what serialises the teardown. A delete
+// that ran on every node would put several destructive handlers on one volume's
+// objects at once, which is what the synchronous path did when a caller retried.
+func (r *VolumeReclaimReaper) Scope() vm.ReaperScope { return vm.ScopeClusterWide }
+
+// Sweep finishes every recorded deletion it can and reports how many records it
+// retired. A volume whose teardown fails keeps its record and is retried on the
+// next sweep: predastore being slow, saturated or down delays reclamation and
+// must not undo a delete the tenant has already been told succeeded.
+func (r *VolumeReclaimReaper) Sweep(ctx context.Context) (int, error) {
+	if r.svc.provider == nil {
+		return 0, nil
+	}
+
+	// The sweep has no caller account, so each document supplies the account its
+	// own key is under.
+	volumes, err := r.svc.metadata.ListAllVolumes(ctx)
+	if err != nil {
+		return 0, err
+	}
+
+	reclaimed := 0
+	for _, meta := range volumes {
+		select {
+		case <-ctx.Done():
+			return reclaimed, ctx.Err()
+		default:
+		}
+
+		if meta.State != volumeStateDeleting {
+			continue
+		}
+
+		if err := r.svc.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{
+			Versioned: ebsprovider.NewVersioned(),
+			VolumeID:  meta.VolumeID,
+			Handle:    meta.ProviderHandle,
+		}); err != nil {
+			slog.WarnContext(ctx, "volume-reclaim: teardown incomplete, retrying next sweep",
+				"volumeId", meta.VolumeID, "accountId", meta.TenantID, "err", err)
+			continue
+		}
+
+		// Removing the record is the last step by design: while it exists the
+		// obligation is readable, so a sweep interrupted anywhere above resumes
+		// rather than leaving objects nobody is accountable for.
+		if err := r.svc.metadata.DeleteVolume(ctx, meta.TenantID, meta.VolumeID); err != nil && !objectstore.IsNoSuchKeyError(err) {
+			slog.ErrorContext(ctx, "volume-reclaim: failed to remove the volume record",
+				"volumeId", meta.VolumeID, "accountId", meta.TenantID, "err", err)
+			continue
+		}
+
+		slog.InfoContext(ctx, "volume-reclaim: deletion complete",
+			"volumeId", meta.VolumeID, "accountId", meta.TenantID)
+		reclaimed++
+	}
+	return reclaimed, nil
+}

spinifex/providers/ebs/nats.go

@@ -43,9 +43,11 @@ const dataMovementTimeout = 180 * time.Second
 // Everything absent answers from state already held and keeps the caller's
 // timeout, so a subject added without a deliberate decision stays fast.
 var dataMovementSubjects = map[string]struct{}{
-	CreateVolumeSubject: {},
-	ExpandVolumeSubject: {},
-	CopySnapshotSubject: {},
+	CreateVolumeSubject:   {},
+	ExpandVolumeSubject:   {},
+	CopySnapshotSubject:   {},
+	DeleteVolumeSubject:   {},
+	DeleteSnapshotSubject: {},
 }
 
 // timeoutFor sizes a request to the work its subject names. The per-volume

spinifex/providers/ebs/nats_test.go

@@ -713,8 +713,13 @@ func TestTimeoutForSizesEachSubjectToItsWork(t *testing.T) {
 	ownerDescribe, err := GetVolumeOwnerSubject("vol-1")
 	require.NoError(t, err)
 
+	// A delete costs one round trip per object, so its duration is a function
+	// of the volume. On prod a 100 GiB volume's delete took 52 s against a 30 s
+	// budget, and the overrun was reported to the tenant as a 500 for a delete
+	// that was working.
 	slow := []string{
 		CreateVolumeSubject, ExpandVolumeSubject, CopySnapshotSubject,
+		DeleteVolumeSubject, DeleteSnapshotSubject,
 		snapshotCreate, ownerCopy, ownerExpand, ownerCreate,
 	}
 	for _, subject := range slow {
@@ -724,8 +729,7 @@ func TestTimeoutForSizesEachSubjectToItsWork(t *testing.T) {
 
 	fast := []string{
 		CapabilitiesSubject, GetVolumeSubject, ListVolumesSubject,
-		DeleteVolumeSubject, DeleteSnapshotSubject, ListSnapshotsSubject,
-		ownerDescribe,
+		ListSnapshotsSubject, ownerDescribe,
 	}
 	for _, subject := range fast {
 		assert.Equal(t, callerTimeout, p.timeoutFor(subject),

spinifex/providers/ebs/provider.go

@@ -182,6 +182,11 @@ const (
 	VolumeStateCreating  VolumeState = "creating"
 	VolumeStateAvailable VolumeState = "available"
 	VolumeStateInUse     VolumeState = "in-use"
+	// VolumeStateDeleting is a volume whose teardown is owed but not finished.
+	// AWS documents a volume remaining here for several minutes, because the
+	// object removal behind it scales with the volume and no request can carry
+	// that. Every dependant path refuses it.
+	VolumeStateDeleting VolumeState = "deleting"
 )
 
 // Volume contains only facts the control plane can rely on across providers.

spinifex/providers/ebs/viperblock/provider_handlers.go

@@ -892,6 +892,19 @@ func handleDeleteVolume(ctx context.Context, cfg *Config, nc *nats.Conn, msg *na
 		return
 	}
 
+	// Local teardown runs first because its cost is fixed while the object
+	// deletion below scales with the volume. Behind it, a timeout on a large
+	// volume leaves this node's WAL and dirty marker behind for good.
+	if localPath, err := localVolumeDir(cfg.BaseDir, req.VolumeID); err == nil {
+		if err := os.RemoveAll(localPath); err != nil {
+			slog.Error("ebs.provider.volume.delete: failed to remove local volume directory", "volume", req.VolumeID, "path", localPath, "err", err)
+		}
+	}
+
+	// The volume is going, so a marker pinning it to this node pins nothing.
+	// Left behind it would outlive every copy of the data it describes.
+	cfg.purgeVolumeDirty(ctx, req.VolumeID)
+
 	store := providerObjectStoreFactory(cfg)
 	if err := deleteObjectPrefix(ctx, store, cfg.Bucket, req.VolumeID+"-efi/"); err != nil {
 		slog.Error("ebs.provider.volume.delete: failed to delete aux objects", "volume", req.VolumeID, "err", err)
@@ -904,18 +917,6 @@ func handleDeleteVolume(ctx context.Context, cfg *Config, nc *nats.Conn, msg *na
 		return
 	}
 
-	// Delete is permanent: remove any on-disk WAL/checkpoint cache left on
-	// this node regardless of mount-tracking state, mirroring ebs.delete.
-	if localPath, err := localVolumeDir(cfg.BaseDir, req.VolumeID); err == nil {
-		if err := os.RemoveAll(localPath); err != nil {
-			slog.Error("ebs.provider.volume.delete: failed to remove local volume directory", "volume", req.VolumeID, "path", localPath, "err", err)
-		}
-	}
-
-	// The volume is gone, so a marker pinning it to this node pins nothing.
-	// Left behind it would outlive every copy of the data it describes.
-	cfg.purgeVolumeDirty(ctx, req.VolumeID)
-
 	slog.Info("ebs.provider.volume.delete: deleted", "volume", req.VolumeID)
 	respondProvider(ctx, msg, ebsprovider.DeleteVolumeResponse{Versioned: ebsprovider.NewVersioned()})
 }

spinifex/providers/ebs/viperblock/provider_handlers_test.go

@@ -802,6 +802,54 @@ func TestProviderHandlers_DeleteVolume_MountedVolumeRefused(t *testing.T) {
 	assert.NoError(t, statErr, "a refused delete must leave the NBD socket alone")
 }
 
+// listFailingObjectStore fails every listing, which is the shape of a blob
+// node that stops answering partway through a delete.
+type listFailingObjectStore struct {
+	objectstore.ObjectStore
+
+	err error
+}
+
+func (s *listFailingObjectStore) ListObjectsV2(context.Context, *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) {
+	return nil, s.err
+}
+
+// TestProviderHandlers_DeleteVolume_LocalStateGoesBeforeTheObjects pins the
+// ordering a 100 GiB volume on prod needed. Object deletion costs one round
+// trip per object, so it is the half that runs out of time; behind it the local
+// WAL and the dirty marker are never reached and stay on the node for good.
+func TestProviderHandlers_DeleteVolume_LocalStateGoesBeforeTheObjects(t *testing.T) {
+	_, natsURL := setupEmbeddedNATS(t)
+	cfg := setupTestConfig(t, natsURL)
+	nc := startProviderSubjects(t, cfg, natsURL)
+
+	prev := providerObjectStoreFactory
+	providerObjectStoreFactory = func(*Config) objectstore.ObjectStore {
+		return &listFailingObjectStore{
+			ObjectStore: objectstore.NewMemoryObjectStore(),
+			err:         errors.New("dial node 6: quic dial 10.10.8.5:6660: timeout"),
+		}
+	}
+	t.Cleanup(func() { providerObjectStoreFactory = prev })
+
+	const volumeName = "vol-deleteordering1"
+	localPath, err := localVolumeDir(cfg.BaseDir, volumeName)
+	require.NoError(t, err)
+	require.NoError(t, os.MkdirAll(localPath, 0o750))
+	require.NoError(t, os.WriteFile(filepath.Join(localPath, "wal.0000000001.bin"), []byte("unflushed"), 0o600))
+
+	body := marshalRequest(t, map[string]any{"schema_version": ebsprovider.SchemaVersion, "volume_id": volumeName})
+	msg := requestProvider(t, nc, ebsprovider.DeleteVolumeSubject, body)
+
+	var resp ebsprovider.DeleteVolumeResponse
+	require.NoError(t, json.Unmarshal(msg.Data, &resp))
+	require.NotNil(t, resp.Error, "an unreachable object store must be reported")
+
+	_, statErr := os.Stat(localPath)
+	assert.True(t, os.IsNotExist(statErr),
+		"local teardown is fixed-cost and must not sit behind the half that scales with the volume")
+}
+
 // TestProviderHandlers_PublishVolume_IdempotentRepublish covers the double-
 // writer hazard this provider boundary exists to prevent: republishing a
 // volume this node already has mounted must return the existing attachment

spinifex/runtime/daemon/daemon.go

@@ -2448,6 +2448,7 @@ func (d *Daemon) startCluster() error {
 		}
 		if d.volumeService != nil {
 			reapers = append(reapers, d.volumeService.NewVolumeLeakReaper(d.leakedVolumeInstances))
+			reapers = append(reapers, d.volumeService.NewVolumeReclaimReaper())
 		}
 		if d.eksService != nil {
 			reapers = append(reapers, d.eksService.NewBillableReaper(d.nodeRunningVMs))

spinifex/runtime/daemon/daemon_handlers_volume_test.go

@@ -258,6 +258,60 @@ func TestAttachVolume_InUseDifferentInstance(t *testing.T) {
 	assert.Contains(t, string(resp.Data), "VolumeInUse")
 }
 
+// TestAttachVolume_DeletingVolumeRefused pins the half of the delete contract
+// that is not about the delete itself: a volume whose teardown is already owed
+// must take no new dependant, because the objects behind it are being removed.
+// DeleteVolume records the obligation and returns, so between the call and the
+// reclaimer finishing it the volume is detached and would otherwise look
+// attachable.
+func TestAttachVolume_DeletingVolumeRefused(t *testing.T) {
+	daemon, store := createFullTestDaemonWithStore(t, sharedNATSURL)
+
+	instanceID := "i-attach-vol-deleting"
+	volumeID := "vol-deleting-attach"
+
+	instance := &vm.VM{
+		ID:           instanceID,
+		Status:       vm.StateRunning,
+		AccountID:    testAccountID,
+		InstanceType: getTestInstanceType(t),
+		Instance:     &ec2.Instance{},
+		QMPClient:    &qmp.QMPClient{},
+	}
+	daemon.vmMgr.Insert(instance)
+
+	seedVolumeConfig(t, daemon, store, ebsmetadata.Volume{
+		VolumeID:    volumeID,
+		CapacityGiB: 10,
+		State:       "deleting",
+		TenantID:    testAccountID,
+	})
+
+	sub, err := daemon.natsConn.Subscribe(
+		fmt.Sprintf("ec2.cmd.%s", instanceID),
+		daemon.handleEC2Events,
+	)
+	require.NoError(t, err)
+	defer sub.Unsubscribe()
+
+	command := ec2v1.EC2InstanceCommand{
+		ID:               instanceID,
+		Attributes:       ec2v1.EC2CommandAttributes{AttachVolume: true},
+		AttachVolumeData: &ec2v1.AttachVolumeData{VolumeID: volumeID},
+	}
+	cmdData, err := json.Marshal(command)
+	require.NoError(t, err)
+
+	resp, err := natsRequest(daemon.natsConn,
+		fmt.Sprintf("ec2.cmd.%s", instanceID),
+		cmdData,
+		5*time.Second,
+	)
+	require.NoError(t, err)
+	assert.Contains(t, string(resp.Data), "VolumeInUse",
+		"a volume already recorded for deletion must not take a new attachment")
+}
+
 // TestAttachVolume_IdempotentSameInstance_DeviceMismatch verifies that a
 // same-instance re-attach requesting a DIFFERENT device than the one
 // already attached is treated as a real CSI conflict — AWS returns

spinifex/runtime/daemon/daemon_test.go

@@ -1597,6 +1597,10 @@ func TestInstanceCleanerAdapter_DeleteVolumes_BootVolumeDeletedAfterAttachmentCl
 	err = cleaner.DeleteVolumes(instance)
 	require.NoError(t, err, "the still-attached boot volume must be deleted, not rejected as VolumeInUse")
 
+	// The delete records the obligation and returns, so the reclaimer is what
+	// retires the document.
+	_, err = daemon.volumeService.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
 	_, err = daemon.volumeService.GetVolumeMetadata(testAccountID, volumeID)
 	require.Error(t, err, "the volume must actually be deleted")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)
@@ -1745,6 +1749,9 @@ func TestTerminatedTeardownReaper_SelfHealsFailedVolumeTeardown(t *testing.T) {
 	_, err = reaper.Sweep(context.Background())
 	require.NoError(t, err)
 
+	// The retried delete records the obligation; the reclaimer retires it.
+	_, err = daemon.volumeService.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
 	_, err = daemon.volumeService.GetVolumeMetadata(testAccountID, volumeID)
 	require.Error(t, err, "the retried delete must actually remove the volume")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)