mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-12 23:09:33 +01:00
volume: skip VolumeDelete when its RPC was cancelled before the delete (#12586)
* volume: skip VolumeDelete when its RPC was cancelled before the delete A caller that cancels (a lost tiering lease, an expired deadline) still had its admitted delete run in the Go server, including one that waited behind the copy guard. The Rust server drops a cancelled handler, so return the context error before the delete starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume: wait until the queued delete blocks on the guard before cancelling Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6388dfca9b
commit
a2ffdce2ec
2 files changed
+63
No files matched your search
@@ -237,6 +237,11 @@ func (vs *VolumeServer) VolumeDelete(ctx context.Context, req *volume_server_pb.
|
|||||||
if guard.copying {
|
if guard.copying {
|
||||||
return resp, status.Errorf(codes.Unavailable, "volume %d is being copied", req.VolumeId)
|
return resp, status.Errorf(codes.Unavailable, "volume %d is being copied", req.VolumeId)
|
||||||
}
|
}
|
||||||
|
// Match the Rust server, which drops a cancelled RPC before it runs.
|
||||||
|
if err := ctx.Err(); err != nil {
|
||||||
|
glog.Warningf("volume delete %v not started: %v", req, err)
|
||||||
|
return resp, status.FromContextError(err).Err()
|
||||||
|
}
|
||||||
|
|
||||||
err := vs.store.DeleteVolume(needle.VolumeId(req.VolumeId), req.OnlyEmpty, req.OnlyGarbage, req.KeepRemoteData)
|
err := vs.store.DeleteVolume(needle.VolumeId(req.VolumeId), req.OnlyEmpty, req.OnlyGarbage, req.KeepRemoteData)
|
||||||
|
|
||||||
|
|||||||
@@ -1,16 +1,20 @@
|
|||||||
package weed_server
|
package weed_server
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"runtime"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"google.golang.org/grpc/codes"
|
"google.golang.org/grpc/codes"
|
||||||
"google.golang.org/grpc/status"
|
"google.golang.org/grpc/status"
|
||||||
|
|
||||||
"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
|
"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
|
||||||
"github.com/seaweedfs/seaweedfs/weed/storage"
|
"github.com/seaweedfs/seaweedfs/weed/storage"
|
||||||
|
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
@@ -39,3 +43,57 @@ func TestVolumeDeleteMapsAbsentStoreVolumeToNotFound(t *testing.T) {
|
|||||||
_, err := vs.VolumeDelete(context.Background(), &volume_server_pb.VolumeDeleteRequest{VolumeId: 17})
|
_, err := vs.VolumeDelete(context.Background(), &volume_server_pb.VolumeDeleteRequest{VolumeId: 17})
|
||||||
assert.Equal(t, codes.NotFound, status.Code(err), err)
|
assert.Equal(t, codes.NotFound, status.Code(err), err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestVolumeDeleteSkipsCancelledRPC(t *testing.T) {
|
||||||
|
vid := needle.VolumeId(1)
|
||||||
|
vs := newVacuumCommitTestServer(t, 0, vid)
|
||||||
|
|
||||||
|
cancelled, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
_, err := vs.VolumeDelete(cancelled, &volume_server_pb.VolumeDeleteRequest{VolumeId: uint32(vid)})
|
||||||
|
assert.Equal(t, codes.Canceled, status.Code(err), err)
|
||||||
|
require.NotNil(t, vs.store.GetVolume(vid), "a cancelled delete must leave the volume")
|
||||||
|
|
||||||
|
expired, cancelExpired := context.WithDeadline(context.Background(), time.Now().Add(-time.Second))
|
||||||
|
defer cancelExpired()
|
||||||
|
_, err = vs.VolumeDelete(expired, &volume_server_pb.VolumeDeleteRequest{VolumeId: uint32(vid)})
|
||||||
|
assert.Equal(t, codes.DeadlineExceeded, status.Code(err), err)
|
||||||
|
require.NotNil(t, vs.store.GetVolume(vid), "an expired delete must leave the volume")
|
||||||
|
|
||||||
|
_, err = vs.VolumeDelete(context.Background(), &volume_server_pb.VolumeDeleteRequest{VolumeId: uint32(vid)})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Nil(t, vs.store.GetVolume(vid))
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestVolumeDeleteCancelledWhileQueuedBehindGuard(t *testing.T) {
|
||||||
|
vid := needle.VolumeId(1)
|
||||||
|
vs := newVacuumCommitTestServer(t, 0, vid)
|
||||||
|
|
||||||
|
guard := vs.volumeCopyGuard(uint32(vid))
|
||||||
|
guard.Lock()
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
done := make(chan error, 1)
|
||||||
|
go func() {
|
||||||
|
_, err := vs.VolumeDelete(ctx, &volume_server_pb.VolumeDeleteRequest{VolumeId: uint32(vid)})
|
||||||
|
done <- err
|
||||||
|
}()
|
||||||
|
require.Eventually(t, func() bool {
|
||||||
|
buf := make([]byte, 1<<20)
|
||||||
|
for _, g := range bytes.Split(buf[:runtime.Stack(buf, true)], []byte("\n\n")) {
|
||||||
|
if bytes.Contains(g, []byte("(*VolumeServer).VolumeDelete")) && bytes.Contains(g, []byte("sync.(*Mutex).Lock")) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}, 10*time.Second, time.Millisecond)
|
||||||
|
cancel()
|
||||||
|
guard.Unlock()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case err := <-done:
|
||||||
|
assert.Equal(t, codes.Canceled, status.Code(err), err)
|
||||||
|
case <-time.After(10 * time.Second):
|
||||||
|
t.Fatal("VolumeDelete did not return")
|
||||||
|
}
|
||||||
|
assert.NotNil(t, vs.store.GetVolume(vid), "a delete cancelled while queued must leave the volume")
|
||||||
|
}
|
||||||
Reference in new issue
Block a user