s3: tear down the emptied .versions directory on last-version delete; drain existing residue (#10278)

* s3: routed last-version delete removes the emptied .versions directory

The routed versioned delete (routedDeleteSpecificVersion) repoints the
latest pointer and deletes the version file, but unlike the lock-path
fallback (updateLatestVersionAfterDeletion) it never tears down the
.versions/ directory it just emptied. The residue keeps the key's read
path in the self-heal rescan loop: every GET of the deleted key logs
event=surfaced plus a GetObject error until the background
EmptyFolderCleaner gets to the directory — at least two minutes away on
its delay queue, and possibly never (the queue is in-memory, bounded,
and gated on the bucket's allow-empty-folders policy). Veeam's lock
arbitration probes deleted lock keys continuously, so those windows are
always open and the log spam is chronic.

ObjectMutation DELETE gains remove_empty_parent: after the child delete,
the filer best-effort removes the parent directory in the same locked
transaction. Non-recursive on purpose — a concurrent write that lands a
new version fails the removal instead of being lost with it. The routed
last-version delete sets it on the version-file DELETE, matching the
lock-path fallback's contract.

Claude-Session: https://claude.ai/code/session_014mMYAHXZySkCCUfpRFNtSv

* s3: drain empty .versions residue on read heal and in s3.versions.audit

Directories already stranded by pre-teardown deletes (or dropped from
the EmptyFolderCleaner's bounded in-memory queue) previously re-entered
the self-heal rescan on every GET forever: the heal only cleared the
pointer and nothing ever removed the directory, and s3.versions.audit
counted the state as clean.

When the heal rescan finds no remaining version, remove the directory
outright (non-recursive, so orphan children still block and fall back to
the pointer clear) and log event=healed mode=empty_dir_removed; the next
GET takes the clean not-found path. The audit gains an empty category so
the residue is visible, and -heal removes such directories in bulk.

Claude-Session: https://claude.ai/code/session_014mMYAHXZySkCCUfpRFNtSv
This commit is contained in:
Chris Lu
2026-07-08 18:50:54 -07:00
committed by GitHub
parent 11fd20e2b0
commit a9cfbd8d3a
9 changed files with 191 additions and 35 deletions
@@ -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
+1
View File
@@ -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
+26 -17
View File
@@ -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" +
+1 -1
View File
@@ -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
@@ -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)
+16 -7
View File
@@ -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.
+15 -3
View File
@@ -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)
@@ -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) {
+47 -6
View File
@@ -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 {