Files
seaweedfs/weed/s3api/s3tables/s3tablestest/memfiler.go
T
d6ec6bc4e7 fix(s3tables): let DeleteTableBucket remove empty leftover directories (#12307)
* s3tables: let DeleteTableBucket remove empty leftover directories

Page through the bucket instead of reading one page, and check each bare
directory a deleted namespace left behind for files: empty ones go with the
bucket, one holding files refuses with 409 naming it.

The delete claims the bucket before scanning, and CreateNamespace refuses a
claimed bucket and confirms it again after its write, so a namespace created
meanwhile is seen by the scan or turned away instead of deleted with the bucket.
The bucket is removed only while it still carries that claim, otherwise 409.
A delete that refuses lifts the claim, retrying a failed release, since a claim
left behind refuses every create in the bucket.
The new namespace stays marked unfinished until that check passes, so no table
or view is created in one that gets backed out; a create takes over a namespace
still carrying the mark. Both checks accept only the bucket the create was
authorized against, by its creation time: one recreated under the same name
answers 404 and gets nothing written into it.

The removal is not recursive. Each leftover is claimed first, only while it is
still the directory the scan saw, and goes only while it holds no file or
catalog record; the bucket goes only while nothing is left in it. A file
uploaded or a namespace taken back after the scan is refused with 409 instead
of deleted.

* s3tables: keep directory-backed S3 objects in DeleteTableBucket

* s3tables: keep a file that replaced a leftover directory in DeleteTableBucket

* s3tables: claim only the authorized bucket in DeleteTableBucket

* s3tables: assert the prefix-object mark when claiming a leftover directory

* s3tables: hide unfinished namespaces from readers and DeleteNamespace

* s3tables: stop CreateNamespace on any failed bucket check

* s3tables: retry lifting claims on leftover directories

* s3vectors: declare the index namespace again after losing it to a concurrent declare

CreateNamespace now writes a namespace marked unfinished and confirms it
after checking the bucket. A second CreateIndex that declares the namespace
in that window takes the marked record over, and the first one's
confirmation answers 409, which failed its CreateIndex. Declare it again,
a bounded number of times, when the create answers a conflict.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3tables: name an unfinished namespace that keeps a bucket from being deleted

A CreateNamespace that dies between its write and its confirmation leaves
a namespace marked unfinished. Listings and GetNamespace hide it and
DeleteNamespace answers that it does not exist, while DeleteTableBucket
counts it, so the bucket could not be deleted and nothing said why. Name
it in the BucketNotEmpty answer; creating it again takes it over.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3tables: read a refused empty-directory delete with filer.DeleteEntryError

Use the filer's own reading of a non-recursive delete's answer instead of
a copy of its message prefix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3vectors: release the held create when the concurrent-declare test fails

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3vectors: buffer the held create's result so it can exit after a failure

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3tables: retry lifting a refused namespace delete's claim

A claim left on a namespace refuses every create in it, the same harm the
bucket's claim does, so its release retries too. With no one-shot caller
left, the retrying release takes the releaseDeleteClaim name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3tables: keep the data under a hidden directory on bucket delete

The scan filed every hidden entry by name for a recursive delete, so .hidden/data went with the bucket. A hidden directory waits with the leftovers instead: the walk refuses one that holds a file or an object, and removeEmptyTree takes only one that is still bare and empty. A hidden file is removed by name as before.

---------

Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
2026-11-01 13:43:15 -08:00

289 lines
10 KiB
Go

// Package s3tablestest provides an in-memory filer for driving S3 Tables and
// Lance namespace operations end-to-end without a live cluster.
package s3tablestest
import (
"bytes"
"context"
"net"
"path"
"sort"
"strings"
"sync"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/status"
)
// MemFiler is an in-memory filer used to drive Manager operations
// end-to-end without a live cluster.
type MemFiler struct {
filer_pb.UnimplementedSeaweedFilerServer
// mu guards entries. The real filer serves concurrent RPCs, and a test that
// races writers against each other - the point of an exclusive create - hits
// this map from several goroutines at once.
mu sync.RWMutex
entries map[string]map[string]*filer_pb.Entry // dir -> name -> entry
Client filer_pb.SeaweedFilerClient
// BeforeUpdate runs once, at the start of the next UpdateEntry, so a test
// can land a competing write in a handler's read-to-write window.
BeforeUpdate func()
// BeforeDelete runs once before the next DeleteEntry, so a test can land a
// competing write in a handler's check-to-delete window.
BeforeDelete func()
// OnLookup runs at the start of every LookupDirectoryEntry with the target
// path, so a test can land a competing write between two reads.
OnLookup func(dir, name string)
// OnUpdate runs at the start of every UpdateEntry with the target path, so
// a test can land a competing write at a chosen step of a multi-update
// operation.
OnUpdate func(dir, name string)
// RejectDelete returns the reason DeleteEntry refuses a path, answered the
// way the filer answers one: no transport error, the reason in the response.
RejectDelete func(dir, name string) string
}
func newMemFiler() *MemFiler {
return &MemFiler{entries: make(map[string]map[string]*filer_pb.Entry)}
}
func (f *MemFiler) Get(dir, name string) *filer_pb.Entry {
f.mu.RLock()
defer f.mu.RUnlock()
if d, ok := f.entries[dir]; ok {
return d[name]
}
return nil
}
func (f *MemFiler) Put(dir, name string, extended map[string][]byte) {
f.mu.Lock()
defer f.mu.Unlock()
if _, ok := f.entries[dir]; !ok {
f.entries[dir] = make(map[string]*filer_pb.Entry)
}
f.entries[dir][name] = &filer_pb.Entry{Name: name, IsDirectory: true, Extended: extended}
}
// PutFile adds a file entry with an explicit modification time, which callers
// that age entries out (orphan cleanup, expiry) need in order to see them.
func (f *MemFiler) PutFile(dir, name string, mtime time.Time) {
f.mu.Lock()
defer f.mu.Unlock()
if _, ok := f.entries[dir]; !ok {
f.entries[dir] = make(map[string]*filer_pb.Entry)
}
f.entries[dir][name] = &filer_pb.Entry{
Name: name,
Attributes: &filer_pb.FuseAttributes{Mtime: mtime.Unix(), Crtime: mtime.Unix()},
}
}
func (f *MemFiler) LookupDirectoryEntry(_ context.Context, req *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) {
if hook := f.OnLookup; hook != nil {
hook(req.Directory, req.Name)
}
if e := f.Get(req.Directory, req.Name); e != nil {
return &filer_pb.LookupDirectoryEntryResponse{Entry: e}, nil
}
// Carry the sentinel text so filer_pb.LookupEntry maps it to ErrNotFound.
return nil, status.Errorf(codes.NotFound, "%s: %s/%s", filer_pb.ErrNotFound.Error(), req.Directory, req.Name)
}
// ListEntries honours prefix, start-from and limit the way the real filer does.
// A harness that ignores them makes a paginating caller re-read the first page
// forever, which looks like duplicated entries rather than a broken listing.
func (f *MemFiler) ListEntries(req *filer_pb.ListEntriesRequest, stream grpc.ServerStreamingServer[filer_pb.ListEntriesResponse]) error {
f.mu.RLock()
d, ok := f.entries[req.Directory]
names := make([]string, 0, len(d))
snapshot := make(map[string]*filer_pb.Entry, len(d))
for name, entry := range d {
names = append(names, name)
snapshot[name] = entry
}
f.mu.RUnlock()
if !ok {
return nil
}
sort.Strings(names)
sent := uint32(0)
for _, name := range names {
if req.Prefix != "" && !strings.HasPrefix(name, req.Prefix) {
continue
}
if req.StartFromFileName != "" {
if name < req.StartFromFileName {
continue
}
if name == req.StartFromFileName && !req.InclusiveStartFrom {
continue
}
}
if err := stream.Send(&filer_pb.ListEntriesResponse{Entry: snapshot[name]}); err != nil {
return err
}
sent++
if req.Limit > 0 && sent >= req.Limit {
return nil
}
}
return nil
}
func (f *MemFiler) CreateEntry(_ context.Context, req *filer_pb.CreateEntryRequest) (*filer_pb.CreateEntryResponse, error) {
f.mu.Lock()
defer f.mu.Unlock()
if _, ok := f.entries[req.Directory]; !ok {
f.entries[req.Directory] = make(map[string]*filer_pb.Entry)
}
// O_EXCL is the filer's put-if-not-exists. Ignoring it here would let a test
// that races two writers see both of them win.
if _, exists := f.entries[req.Directory][req.Entry.Name]; exists && req.OExcl {
return &filer_pb.CreateEntryResponse{ErrorCode: filer_pb.FilerError_ENTRY_ALREADY_EXISTS}, nil
}
f.entries[req.Directory][req.Entry.Name] = req.Entry
return &filer_pb.CreateEntryResponse{}, nil
}
func (f *MemFiler) UpdateEntry(_ context.Context, req *filer_pb.UpdateEntryRequest) (*filer_pb.UpdateEntryResponse, error) {
// The hooks run before the lock is taken: their whole purpose is to land a
// competing write in the read-to-write window, and that write needs the lock.
if hook := f.OnUpdate; hook != nil {
hook(req.Directory, req.Entry.Name)
}
if hook := f.BeforeUpdate; hook != nil {
f.BeforeUpdate = nil
hook()
}
f.mu.Lock()
defer f.mu.Unlock()
// The real filer looks the entry up before anything else and answers a
// missing one with codes.NotFound, not a failed precondition.
existing := f.entries[req.Directory][req.Entry.Name]
if existing == nil {
return nil, status.Errorf(codes.NotFound, "not found %s: %v", path.Join(req.Directory, req.Entry.Name), filer_pb.ErrNotFound)
}
// The real filer will not write a directory entry over a file or the
// reverse; without it here a write that raced a replacement would turn the
// replacement into the entry it never was.
if existing.IsDirectory != req.Entry.IsDirectory {
return nil, status.Errorf(codes.Unknown, "%s: existing entry type differs", path.Join(req.Directory, req.Entry.Name))
}
// The real filer validates ExpectedExtended under the per-path lock; without
// it here a lost update would look like a success.
for key, expected := range req.ExpectedExtended {
actual := existing.Extended[key]
if !bytes.Equal(actual, expected) {
return nil, status.Errorf(codes.FailedPrecondition, "extended attribute %q changed", key)
}
}
if _, ok := f.entries[req.Directory]; !ok {
f.entries[req.Directory] = make(map[string]*filer_pb.Entry)
}
f.entries[req.Directory][req.Entry.Name] = req.Entry
return &filer_pb.UpdateEntryResponse{}, nil
}
func (f *MemFiler) DeleteEntry(_ context.Context, req *filer_pb.DeleteEntryRequest) (*filer_pb.DeleteEntryResponse, error) {
if hook := f.BeforeDelete; hook != nil {
f.BeforeDelete = nil
hook()
}
if reject := f.RejectDelete; reject != nil {
if reason := reject(req.Directory, req.Name); reason != "" {
return &filer_pb.DeleteEntryResponse{Error: reason}, nil
}
}
f.mu.Lock()
defer f.mu.Unlock()
// The real filer evaluates ExpectedExtended under the path lock and refuses
// the delete when the entry no longer matches what the caller captured. It
// does so before anything else, so a directory that changed hands is a
// changed entry whether or not it also holds something.
for key, expected := range req.ExpectedExtended {
var actual []byte
if d, ok := f.entries[req.Directory]; ok {
if existing := d[req.Name]; existing != nil {
actual = existing.Extended[key]
}
}
if !bytes.Equal(actual, expected) {
return nil, status.Errorf(codes.FailedPrecondition, "extended attribute %q changed", key)
}
}
// The real filer then refuses a non-recursive delete of a non-empty folder.
if child := path.Join(req.Directory, req.Name); !req.IsRecursive && len(f.entries[child]) > 0 {
return &filer_pb.DeleteEntryResponse{Error: "fail to delete non-empty folder: " + child}, nil
}
if d, ok := f.entries[req.Directory]; ok {
delete(d, req.Name)
}
// Honor recursive data deletion so a regression that wipes the table directory
// also drops its metadata/ and data/ children (the data-loss this guards against).
if req.IsRecursive && req.IsDeleteData {
child := path.Join(req.Directory, req.Name)
for dir := range f.entries {
if dir == child || strings.HasPrefix(dir, child+"/") {
delete(f.entries, dir)
}
}
}
return &filer_pb.DeleteEntryResponse{}, nil
}
// GetFilerConfiguration answers with the defaults, so operations that resolve
// the buckets directory before touching an entry work against this filer.
func (f *MemFiler) GetFilerConfiguration(_ context.Context, _ *filer_pb.GetFilerConfigurationRequest) (*filer_pb.GetFilerConfigurationResponse, error) {
return &filer_pb.GetFilerConfigurationResponse{DirBuckets: s3_constants.DefaultBucketsPath}, nil
}
func (f *MemFiler) Ping(_ context.Context, _ *filer_pb.PingRequest) (*filer_pb.PingResponse, error) {
now := time.Now().UnixNano()
return &filer_pb.PingResponse{StartTimeNs: now, RemoteTimeNs: now, StopTimeNs: now}, nil
}
func Start(t *testing.T) *MemFiler {
t.Helper()
fs := newMemFiler()
listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("start filer: %v", err)
}
server := grpc.NewServer()
filer_pb.RegisterSeaweedFilerServer(server, fs)
go func() { _ = server.Serve(listener) }()
t.Cleanup(server.GracefulStop)
conn, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("start filer: %v", err)
}
t.Cleanup(func() { _ = conn.Close() })
fs.Client = filer_pb.NewSeaweedFilerClient(conn)
deadline := time.Now().Add(5 * time.Second)
for {
pingCtx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
_, err := fs.Client.Ping(pingCtx, &filer_pb.PingRequest{})
cancel()
if err == nil {
break
}
if time.Now().After(deadline) {
t.Fatalf("filer not ready: %v", err)
}
time.Sleep(10 * time.Millisecond)
}
return fs
}