diff --git a/other/java/client/src/main/proto/filer.proto b/other/java/client/src/main/proto/filer.proto index 2b068967c..070874e2e 100644 --- a/other/java/client/src/main/proto/filer.proto +++ b/other/java/client/src/main/proto/filer.proto @@ -325,6 +325,7 @@ message ObjectMutation { bool set_content = 10; // PATCH_EXTENDED: replace Entry.content with content bytes content = 11; // PATCH_EXTENDED: new Entry.content when set_content bool touch_mtime = 12; // PATCH_EXTENDED: set the entry's Mtime to now (e.g. a metadata-replace copy) + bool remove_empty_parent = 13; // DELETE: also remove the parent directory when the delete leaves it empty (best-effort) } // Recompute re-derives a pointer entry (directory/name on the mutation) from the diff --git a/weed/pb/filer.proto b/weed/pb/filer.proto index 281c36c3f..ab9e0b285 100644 --- a/weed/pb/filer.proto +++ b/weed/pb/filer.proto @@ -325,6 +325,7 @@ message ObjectMutation { bool set_content = 10; // PATCH_EXTENDED: replace Entry.content with content bytes content = 11; // PATCH_EXTENDED: new Entry.content when set_content bool touch_mtime = 12; // PATCH_EXTENDED: set the entry's Mtime to now (e.g. a metadata-replace copy) + bool remove_empty_parent = 13; // DELETE: also remove the parent directory when the delete leaves it empty (best-effort) } // Recompute re-derives a pointer entry (directory/name on the mutation) from the diff --git a/weed/pb/filer_pb/filer.pb.go b/weed/pb/filer_pb/filer.pb.go index 928f9d420..51d92a248 100644 --- a/weed/pb/filer_pb/filer.pb.go +++ b/weed/pb/filer_pb/filer.pb.go @@ -1476,21 +1476,22 @@ func (x *WriteCondition) GetClauses() []*WriteCondition_Clause { // several RPCs. Data-bearing writes (entries with chunks) should be written // before the transaction; mutations here are metadata-scoped. type ObjectMutation struct { - state protoimpl.MessageState `protogen:"open.v1"` - Type ObjectMutation_Type `protobuf:"varint,1,opt,name=type,proto3,enum=filer_pb.ObjectMutation_Type" json:"type,omitempty"` - Directory string `protobuf:"bytes,2,opt,name=directory,proto3" json:"directory,omitempty"` - Name string `protobuf:"bytes,3,opt,name=name,proto3" json:"name,omitempty"` // entry name for DELETE / PATCH_EXTENDED / RECOMPUTE_LATEST (the pointer entry) - Entry *Entry `protobuf:"bytes,4,opt,name=entry,proto3" json:"entry,omitempty"` // full entry for PUT - SetExtended map[string][]byte `protobuf:"bytes,5,rep,name=set_extended,json=setExtended,proto3" json:"set_extended,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` // PATCH_EXTENDED: keys to set - DeleteExtended []string `protobuf:"bytes,6,rep,name=delete_extended,json=deleteExtended,proto3" json:"delete_extended,omitempty"` // PATCH_EXTENDED: keys to remove - IsDeleteData bool `protobuf:"varint,7,opt,name=is_delete_data,json=isDeleteData,proto3" json:"is_delete_data,omitempty"` // DELETE: also delete chunk data - IsRecursive bool `protobuf:"varint,8,opt,name=is_recursive,json=isRecursive,proto3" json:"is_recursive,omitempty"` // DELETE: recurse into a directory - Recompute *Recompute `protobuf:"bytes,9,opt,name=recompute,proto3" json:"recompute,omitempty"` // RECOMPUTE_LATEST parameters - SetContent bool `protobuf:"varint,10,opt,name=set_content,json=setContent,proto3" json:"set_content,omitempty"` // PATCH_EXTENDED: replace Entry.content with content - Content []byte `protobuf:"bytes,11,opt,name=content,proto3" json:"content,omitempty"` // PATCH_EXTENDED: new Entry.content when set_content - TouchMtime bool `protobuf:"varint,12,opt,name=touch_mtime,json=touchMtime,proto3" json:"touch_mtime,omitempty"` // PATCH_EXTENDED: set the entry's Mtime to now (e.g. a metadata-replace copy) - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + state protoimpl.MessageState `protogen:"open.v1"` + Type ObjectMutation_Type `protobuf:"varint,1,opt,name=type,proto3,enum=filer_pb.ObjectMutation_Type" json:"type,omitempty"` + Directory string `protobuf:"bytes,2,opt,name=directory,proto3" json:"directory,omitempty"` + Name string `protobuf:"bytes,3,opt,name=name,proto3" json:"name,omitempty"` // entry name for DELETE / PATCH_EXTENDED / RECOMPUTE_LATEST (the pointer entry) + Entry *Entry `protobuf:"bytes,4,opt,name=entry,proto3" json:"entry,omitempty"` // full entry for PUT + SetExtended map[string][]byte `protobuf:"bytes,5,rep,name=set_extended,json=setExtended,proto3" json:"set_extended,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` // PATCH_EXTENDED: keys to set + DeleteExtended []string `protobuf:"bytes,6,rep,name=delete_extended,json=deleteExtended,proto3" json:"delete_extended,omitempty"` // PATCH_EXTENDED: keys to remove + IsDeleteData bool `protobuf:"varint,7,opt,name=is_delete_data,json=isDeleteData,proto3" json:"is_delete_data,omitempty"` // DELETE: also delete chunk data + IsRecursive bool `protobuf:"varint,8,opt,name=is_recursive,json=isRecursive,proto3" json:"is_recursive,omitempty"` // DELETE: recurse into a directory + Recompute *Recompute `protobuf:"bytes,9,opt,name=recompute,proto3" json:"recompute,omitempty"` // RECOMPUTE_LATEST parameters + SetContent bool `protobuf:"varint,10,opt,name=set_content,json=setContent,proto3" json:"set_content,omitempty"` // PATCH_EXTENDED: replace Entry.content with content + Content []byte `protobuf:"bytes,11,opt,name=content,proto3" json:"content,omitempty"` // PATCH_EXTENDED: new Entry.content when set_content + TouchMtime bool `protobuf:"varint,12,opt,name=touch_mtime,json=touchMtime,proto3" json:"touch_mtime,omitempty"` // PATCH_EXTENDED: set the entry's Mtime to now (e.g. a metadata-replace copy) + RemoveEmptyParent bool `protobuf:"varint,13,opt,name=remove_empty_parent,json=removeEmptyParent,proto3" json:"remove_empty_parent,omitempty"` // DELETE: also remove the parent directory when the delete leaves it empty (best-effort) + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *ObjectMutation) Reset() { @@ -1607,6 +1608,13 @@ func (x *ObjectMutation) GetTouchMtime() bool { return false } +func (x *ObjectMutation) GetRemoveEmptyParent() bool { + if x != nil { + return x.RemoveEmptyParent + } + return false +} + // Recompute re-derives a pointer entry (directory/name on the mutation) from the // current contents of a scanned directory, atomically under the transaction's // lock. It is mechanical: the filer picks the child that sorts first or last by @@ -6928,7 +6936,7 @@ const file_filer_proto_rawDesc = "" + "\x13IF_UNMODIFIED_SINCE\x10\x05\x12\x15\n" + "\x11IF_MODIFIED_SINCE\x10\x06\x12\x19\n" + "\x15IF_EXTENDED_NOT_EQUAL\x10\a\x12\x1c\n" + - "\x18IF_EXTENDED_TIME_ELAPSED\x10\b\"\xf2\x04\n" + + "\x18IF_EXTENDED_TIME_ELAPSED\x10\b\"\xa2\x05\n" + "\x0eObjectMutation\x121\n" + "\x04type\x18\x01 \x01(\x0e2\x1d.filer_pb.ObjectMutation.TypeR\x04type\x12\x1c\n" + "\tdirectory\x18\x02 \x01(\tR\tdirectory\x12\x12\n" + @@ -6944,7 +6952,8 @@ const file_filer_proto_rawDesc = "" + "setContent\x12\x18\n" + "\acontent\x18\v \x01(\fR\acontent\x12\x1f\n" + "\vtouch_mtime\x18\f \x01(\bR\n" + - "touchMtime\x1a>\n" + + "touchMtime\x12.\n" + + "\x13remove_empty_parent\x18\r \x01(\bR\x11removeEmptyParent\x1a>\n" + "\x10SetExtendedEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\fR\x05value:\x028\x01\"E\n" + diff --git a/weed/pb/filer_pb/filer_grpc.pb.go b/weed/pb/filer_pb/filer_grpc.pb.go index 196b3f5e4..0d103b4c5 100644 --- a/weed/pb/filer_pb/filer_grpc.pb.go +++ b/weed/pb/filer_pb/filer_grpc.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: // - protoc-gen-go-grpc v1.6.2 -// - protoc v6.33.4 +// - protoc v7.35.0 // source: filer.proto package filer_pb diff --git a/weed/s3api/s3api_object_versioned_finalize.go b/weed/s3api/s3api_object_versioned_finalize.go index d377285ce..9dd968835 100644 --- a/weed/s3api/s3api_object_versioned_finalize.go +++ b/weed/s3api/s3api_object_versioned_finalize.go @@ -111,6 +111,9 @@ func wormDeleteCondition(worm, bypass bool) *filer_pb.WriteCondition { // rather than a dangling pointer) and deletes the version file. lock_key is the // object (serializing the pointer recompute); for object-lock buckets the // condition gates the delete on the version's WORM guards evaluated on the owner. +// Deleting the last version also removes the emptied .versions/ directory — +// leaving it behind would keep re-triggering the read path's self-heal rescans +// on every GET of the key (Veeam probes its deleted lock objects forever). func (s3a *S3ApiServer) routedDeleteSpecificVersion(owner pb.ServerAddress, bucket, object, versionId string, worm, bypass bool) s3err.ErrorCode { if !isValidVersionID(versionId) { return s3err.ErrInvalidRequest @@ -125,7 +128,7 @@ func (s3a *S3ApiServer) routedDeleteSpecificVersion(owner pb.ServerAddress, buck Condition: cond, Mutations: []*filer_pb.ObjectMutation{ s3a.latestPointerRecompute(bucket, object, isNewFormatVersionId(versionId), versionFileName, false), - {Type: filer_pb.ObjectMutation_DELETE, Directory: versionsPath, Name: versionFileName, IsDeleteData: true}, + {Type: filer_pb.ObjectMutation_DELETE, Directory: versionsPath, Name: versionFileName, IsDeleteData: true, RemoveEmptyParent: true}, }, } resp, err := s3a.objectTxnOnFiler(owner, req) diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index bea7dfb48..0371bf5c1 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -1957,13 +1957,22 @@ func (s3a *S3ApiServer) healStaleLatestVersionPointer(bucket, normalizedObject s } if latestEntry == nil { - // Best-effort clear the stale latest-version pointer so subsequent - // reads short-circuit to ErrNotFound directly instead of replaying - // getLatestObjectVersion's read-retry loop and re-entering self-heal - // on every request. Orphan entries (files in .versions/ that lack - // the version-id extended attribute) remain in place; from the S3 - // API perspective the object is correctly absent. - s3a.clearStaleLatestVersionPointer(bucket, normalizedObject, bucketDir, versionsObjectPath, versionsEntry, "healStaleLatestVersionPointer") + // No version remains: remove the residue directory outright so future + // reads take the clean ErrNotFound path instead of re-entering this + // self-heal (and its rescan + surfaced warning) on every request. The + // non-recursive rm makes this safe against a concurrent PUT — a new + // child fails the rm, and a directory removed just before that PUT's + // version file lands is recreated by the create's parent handling. + if rmErr := s3a.rm(bucketDir, versionsObjectPath, true, false); rmErr == nil { + versioningHealInfof("healed", "bucket=%s key=%s mode=empty_dir_removed", bucket, normalizedObject) + } else { + // Orphan entries (files in .versions/ that lack the version-id + // extended attribute) block the teardown and remain in place for an + // operator; fall back to clearing the stale pointer so reads at + // least short-circuit. From the S3 API perspective the object is + // correctly absent either way. + s3a.clearStaleLatestVersionPointer(bucket, normalizedObject, bucketDir, versionsObjectPath, versionsEntry, "healStaleLatestVersionPointer") + } // Wrap filer_pb.ErrNotFound so callers can distinguish genuine // object-absence (nothing left to promote) from scan failures // (I/O errors during list) via errors.Is. diff --git a/weed/server/filer_grpc_server.go b/weed/server/filer_grpc_server.go index e9e71b6c9..985607d00 100644 --- a/weed/server/filer_grpc_server.go +++ b/weed/server/filer_grpc_server.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "time" "github.com/seaweedfs/seaweedfs/weed/cluster" @@ -391,10 +392,21 @@ func (fs *FilerServer) applyObjectMutation(ctx context.Context, m *filer_pb.Obje case filer_pb.ObjectMutation_DELETE: fullpath := util.NewFullPath(m.Directory, m.Name) err := fs.filer.DeleteEntryMetaAndData(ctx, fullpath, m.IsRecursive, false, m.IsDeleteData, fromOtherCluster, signatures, 0) - if err == filer_pb.ErrNotFound { - return nil + if err != nil && err != filer_pb.ErrNotFound { + return err } - return err + if m.RemoveEmptyParent { + // A parent that exists only to hold this child (e.g. a .versions/ + // directory losing its last version) is torn down in the same locked + // transaction. Best-effort: a non-empty or already-removed parent is + // the expected no-op, and a failed teardown must not fail the + // already-applied delete. + parentErr := fs.filer.DeleteEntryMetaAndData(ctx, util.FullPath(m.Directory), false, false, false, fromOtherCluster, signatures, 0) + if parentErr != nil && parentErr != filer_pb.ErrNotFound && !strings.Contains(parentErr.Error(), filer.MsgFailDelNonEmptyFolder) { + glog.V(1).InfofCtx(ctx, "remove empty parent %s: %v", m.Directory, parentErr) + } + } + return nil case filer_pb.ObjectMutation_PATCH_EXTENDED: fullpath := util.NewFullPath(m.Directory, m.Name) diff --git a/weed/server/filer_grpc_server_object_txn_test.go b/weed/server/filer_grpc_server_object_txn_test.go index 098917d7c..013bf7361 100644 --- a/weed/server/filer_grpc_server_object_txn_test.go +++ b/weed/server/filer_grpc_server_object_txn_test.go @@ -619,6 +619,86 @@ func TestObjectTransactionVersionDeleteWithWorm(t *testing.T) { } } +// Deleting the LAST version tears down the emptied .versions/ directory in the +// same transaction (remove_empty_parent on the DELETE), so a fully-drained key +// leaves no residue to re-trigger the read path's self-heal on every GET. A +// remaining child keeps the directory; a replay after the child is already +// gone still tidies it. +func TestObjectTransactionDeleteRemovesEmptyParent(t *testing.T) { + now := time.Unix(1700000000, 0) + seed := func(withSibling bool) map[string]*filer.Entry { + entries := map[string]*filer.Entry{ + "/buckets/b/obj/.versions": { + Attr: filer.Attr{Inode: 2, Mtime: now, Crtime: now, Mode: 0755 | (1 << 31)}, + Extended: map[string][]byte{"latestName": []byte("v_a"), "latestVid": []byte("v1")}, + }, + "/buckets/b/obj/.versions/v_a": { + Attr: filer.Attr{Inode: 10, Mtime: now, Crtime: now, Mode: 0644}, + Extended: map[string][]byte{"vid": []byte("v1")}, + }, + } + if withSibling { + entries["/buckets/b/obj/.versions/orphan"] = &filer.Entry{ + Attr: filer.Attr{Inode: 11, Mtime: now, Crtime: now, Mode: 0644}, + } + } + return entries + } + // Mirrors routedDeleteSpecificVersion: repoint excluding the dying version, + // then delete it and tidy the parent. + mkReq := func() *filer_pb.ObjectTransactionRequest { + return &filer_pb.ObjectTransactionRequest{ + LockKey: "/buckets/b/obj", + Mutations: []*filer_pb.ObjectMutation{ + {Type: filer_pb.ObjectMutation_RECOMPUTE_LATEST, Directory: "/buckets/b/obj", Name: ".versions", + Recompute: &filer_pb.Recompute{ScanDir: "/buckets/b/obj/.versions", Descending: true, ExcludeName: "v_a", + NameToKey: "latestName", CopyExtended: map[string]string{"latestVid": "vid"}}}, + {Type: filer_pb.ObjectMutation_DELETE, Directory: "/buckets/b/obj/.versions", Name: "v_a", RemoveEmptyParent: true}, + }, + } + } + + // Last version: the emptied directory goes with it. + fs, store := newTxnTestServer(seed(false)) + resp, err := fs.ObjectTransaction(context.Background(), mkReq()) + if err != nil || resp.Error != "" { + t.Fatalf("txn failed: err=%v resp=%q", err, resp.Error) + } + if _, ok := store.entries["/buckets/b/obj/.versions/v_a"]; ok { + t.Errorf("version should be deleted") + } + if _, ok := store.entries["/buckets/b/obj/.versions"]; ok { + t.Errorf(".versions directory emptied by the delete should be removed") + } + + // A remaining child keeps the directory (non-recursive teardown declines). + fs, store = newTxnTestServer(seed(true)) + resp, err = fs.ObjectTransaction(context.Background(), mkReq()) + if err != nil || resp.Error != "" { + t.Fatalf("txn failed: err=%v resp=%q", err, resp.Error) + } + if _, ok := store.entries["/buckets/b/obj/.versions/orphan"]; !ok { + t.Errorf("sibling must survive the teardown attempt") + } + if _, ok := store.entries["/buckets/b/obj/.versions"]; !ok { + t.Errorf(".versions directory with a remaining child must not be removed") + } + + // Replay: the child is already gone, the empty parent is still tidied. + fs, store = newTxnTestServer(map[string]*filer.Entry{ + "/buckets/b/obj/.versions": { + Attr: filer.Attr{Inode: 2, Mtime: now, Crtime: now, Mode: 0755 | (1 << 31)}, + }, + }) + resp, err = fs.ObjectTransaction(context.Background(), mkReq()) + if err != nil || resp.Error != "" { + t.Fatalf("replay txn failed: err=%v resp=%q", err, resp.Error) + } + if _, ok := store.entries["/buckets/b/obj/.versions"]; ok { + t.Errorf("replayed delete should still remove the empty directory") + } +} + // PATCH_EXTENDED with touch_mtime bumps the entry's Mtime (a metadata-replace // copy) while merging Extended. func TestObjectTransactionPatchTouchMtime(t *testing.T) { diff --git a/weed/shell/command_s3_versions_audit.go b/weed/shell/command_s3_versions_audit.go index 07557677e..1be0b16ae 100644 --- a/weed/shell/command_s3_versions_audit.go +++ b/weed/shell/command_s3_versions_audit.go @@ -2,6 +2,7 @@ package shell import ( "context" + "errors" "flag" "fmt" "io" @@ -38,6 +39,8 @@ func (c *commandS3VersionsAudit) Help() string { Veeam/etc. as "Storage not found" on the next GET - orphan (directory has files lacking the version-id extended attr, which the post-delete cleanup path will refuse to rm) + - empty (no pointer and no children — residue of a fully-drained key; + every GET of the key replays the read-side self-heal rescan) Example: # Audit a whole bucket @@ -52,9 +55,10 @@ func (c *commandS3VersionsAudit) Help() string { s3.versions.audit -prefix /buckets/mybucket -heal This command is read-only by default. With -heal, it clears the stale - latest-version pointer on stranded directories; the blob is already - gone, so reads then return NoSuchKey via the clean-miss path instead - of replaying the 10-retry self-heal loop on every request. + latest-version pointer on stranded directories and removes empty ones; + the blob is already gone, so reads then return NoSuchKey via the + clean-miss path instead of replaying the 10-retry self-heal loop on + every request. ` } @@ -81,6 +85,7 @@ func (c *commandS3VersionsAudit) Do(args []string, commandEnv *CommandEnv, write clean uint64 stranded uint64 orphanOnly uint64 + empty uint64 healed uint64 healFailed uint64 ) @@ -117,6 +122,7 @@ func (c *commandS3VersionsAudit) Do(args []string, commandEnv *CommandEnv, write versionsPath := string(parentPath) + "/" + entry.Name pointerSeen := false hasOrphan := false + hasChild := false const auditPageSize = 1024 var startName string for { @@ -128,6 +134,7 @@ func (c *commandS3VersionsAudit) Do(args []string, commandEnv *CommandEnv, write } pageEntries++ lastEntryName = child.Name + hasChild = true hasVersionId := false if child.Extended != nil { if _, ok := child.Extended[s3_constants.ExtVersionIdKey]; ok { @@ -154,12 +161,26 @@ func (c *commandS3VersionsAudit) Do(args []string, commandEnv *CommandEnv, write switch { case pointerFile == "": - // No pointer set. Orphan-only and clean are now mutually + // No pointer set. Empty, orphan-only and clean are mutually // exclusive so the final report's category counts sum to // versionsDirs. - if hasOrphan { + switch { + case !hasChild: + atomic.AddUint64(&empty, 1) + if *verbose { + fmt.Fprintf(writer, "empty: %s\n", versionsPath) + } + if *doHeal { + if err := removeEmptyVersionsDir(ctx, client, parentPath, entry); err != nil { + atomic.AddUint64(&healFailed, 1) + fmt.Fprintf(writer, "heal failed: %s: %v\n", versionsPath, err) + } else { + atomic.AddUint64(&healed, 1) + } + } + case hasOrphan: atomic.AddUint64(&orphanOnly, 1) - } else { + default: atomic.AddUint64(&clean, 1) } case pointerSeen: @@ -190,6 +211,7 @@ func (c *commandS3VersionsAudit) Do(args []string, commandEnv *CommandEnv, write fmt.Fprintf(writer, " clean : %d\n", clean) fmt.Fprintf(writer, " stranded : %d\n", stranded) fmt.Fprintf(writer, " orphan-only : %d\n", orphanOnly) + fmt.Fprintf(writer, " empty : %d\n", empty) if *doHeal { fmt.Fprintf(writer, " healed : %d\n", healed) fmt.Fprintf(writer, " heal failed : %d\n", healFailed) @@ -222,6 +244,25 @@ func healStrandedPointer(ctx context.Context, client filer_pb.SeaweedFilerClient return err } +// removeEmptyVersionsDir deletes a .versions/ directory that holds no +// children: residue of a fully-drained key. Non-recursive on purpose so a +// concurrent write landing a new version fails the delete instead of being +// lost with it. +func removeEmptyVersionsDir(ctx context.Context, client filer_pb.SeaweedFilerClient, parentPath util.FullPath, entry *filer_pb.Entry) error { + resp, err := client.DeleteEntry(ctx, &filer_pb.DeleteEntryRequest{ + Directory: string(parentPath), + Name: entry.Name, + IsDeleteData: true, + }) + if err != nil { + return err + } + if resp.Error != "" { + return errors.New(resp.Error) + } + return nil +} + // filerClientWrapper adapts a raw SeaweedFilerClient to the // filer_pb.FilerClient interface that List / TraverseBfs expect. type filerClientWrapper struct {