filer: keep empty folders that are s3tables catalog entries (#11102)

* s3tables: build the catalog attribute keys from one shared prefix

Every attribute the catalog stores on a bucket, namespace, table or view
entry is spelled out with the same literal prefix. Name it once in
s3_constants so code outside the package can recognize a catalog entry
without repeating the string.

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

* filer: keep empty folders that are s3tables catalog entries

A namespace, table or view is a directory whose extended attributes are
the catalog record. Its files can live elsewhere - a rename moves only
the catalog pointer and leaves the data at the old path, and a view has
no files at all - so an empty one is still a live entry.

Drop a table, then rename another table onto that name: the drop queues
the old table's folders, the rename recreates the name path, and two
minutes later the cleaner deletes it and cascades into the namespace,
losing a table the catalog still lists.

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

* filer: drop a queued cleanup when the folder is created again

A cleanup is queued against the folder that was found empty. If that
folder is deleted and a new one takes its name, the queue entry outlives
the folder it was about and the next pass deletes the replacement. A
drop followed by a rename onto the dropped name does exactly this: the
name path comes back as a live catalog entry two minutes before the
queue is read.

Claude-Session: https://claude.ai/code/session_01GfZsc4cyNB2yr6KYLRv9q1
This commit is contained in:
Chris Lu
2026-09-02 11:49:08 -07:00
committed by GitHub
parent 23a6b8feb5
commit 86761cc7d5
4 changed files with 122 additions and 8 deletions
@@ -239,6 +239,16 @@ func (efc *EmptyFolderCleaner) OnCreateEvent(directory string, entryName string,
if efc.cleanupQueue.Remove(directory) { if efc.cleanupQueue.Remove(directory) {
glog.V(3).Infof("EmptyFolderCleaner: cancelled cleanup for %s due to new entry", directory) glog.V(3).Infof("EmptyFolderCleaner: cancelled cleanup for %s due to new entry", directory)
} }
// A directory that has just been created is a new incarnation, so a cleanup
// queued against the one it replaces would delete it rather than the folder
// that was found empty.
if isDirectory {
recreated := string(util.NewFullPath(directory, entryName))
if efc.cleanupQueue.Remove(recreated) {
glog.V(3).Infof("EmptyFolderCleaner: cancelled cleanup for %s, recreated", recreated)
}
}
} }
// cleanupProcessor runs in background and processes the cleanup queue // cleanupProcessor runs in background and processes the cleanup queue
@@ -529,6 +539,17 @@ func (efc *EmptyFolderCleaner) executeCleanup(folder string, triggeredBy string)
return return
} }
// An S3 Tables catalog entry - a namespace, a table whose files a rename left
// behind at the old path, a view that never had any - is the directory itself,
// so its being empty says nothing about whether the catalog still names it.
if extended, err := efc.filer.GetEntryAttributes(ctx, util.FullPath(folder)); err != nil {
glog.V(2).Infof("EmptyFolderCleaner: error reading attributes of %s: %v", folder, err)
return
} else if isCatalogEntry(extended) {
glog.V(3).Infof("EmptyFolderCleaner: skipping %s (triggered by %s), s3tables catalog entry", folder, triggeredBy)
return
}
// Observe it before the delete rather than after. A delete can fail partway and // Observe it before the delete rather than after. A delete can fail partway and
// still leave the folder gone - the redis stores remove the folder before their // still leave the folder gone - the redis stores remove the folder before their
// parent-list member - so a failure return is not proof that it is still there. // parent-list member - so a failure return is not proof that it is still there.
@@ -617,6 +638,16 @@ func (efc *EmptyFolderCleaner) getBucketCleanupPolicy(ctx context.Context, folde
return bucketPath, autoRemove, "filer", attrValue, nil return bucketPath, autoRemove, "filer", attrValue, nil
} }
// isCatalogEntry reports whether the directory is an s3tables catalog record.
func isCatalogEntry(attrs map[string][]byte) bool {
for key := range attrs {
if strings.HasPrefix(key, s3_constants.ExtS3TablesPrefix) {
return true
}
}
return false
}
func autoRemoveEmptyFoldersEnabled(attrs map[string][]byte) (bool, string) { func autoRemoveEmptyFoldersEnabled(attrs map[string][]byte) (bool, string) {
if attrs == nil { if attrs == nil {
return true, "<no_attrs>" return true, "<no_attrs>"
@@ -1141,6 +1141,85 @@ func TestEmptyFolderCleaner_executeCleanup_directoryMarker(t *testing.T) {
} }
} }
func TestEmptyFolderCleaner_OnCreateEvent_cancelsCleanupForRecreatedDirectory(t *testing.T) {
lockRing := lock_manager.NewLockRing(5 * time.Second)
lockRing.SetSnapshot([]pb.ServerAddress{"filer1:8888"}, 0)
cleaner := &EmptyFolderCleaner{
filer: &mockFilerOps{},
lockRing: lockRing,
host: "filer1:8888",
bucketPath: "/buckets",
enabled: true,
folderCounts: make(map[string]*folderState),
deleted: make(map[string]*deletedFolder),
cleanupQueue: NewCleanupQueue(1000, time.Minute),
maxCountCheck: 1000,
cacheExpiry: time.Minute,
processorSleep: time.Second,
stopCh: make(chan struct{}),
}
// Dropping the table queues the folder its children were removed from, then a
// rename puts a new table back under the same name.
cleaner.OnDeleteEvent("/buckets/warehouse/pedsnet/person", "metadata", true, time.Now())
if cleaner.GetPendingCleanupCount() != 1 {
t.Fatalf("expected the emptied folder to be queued, got %d", cleaner.GetPendingCleanupCount())
}
cleaner.OnCreateEvent("/buckets/warehouse/pedsnet", "person", true)
if cleaner.GetPendingCleanupCount() != 0 {
t.Fatalf("expected the recreated folder to leave the queue, got %d", cleaner.GetPendingCleanupCount())
}
}
func TestEmptyFolderCleaner_executeCleanup_skipsCatalogEntry(t *testing.T) {
lockRing := lock_manager.NewLockRing(5 * time.Second)
lockRing.SetSnapshot([]pb.ServerAddress{"filer1:8888"}, 0)
const namespace = "/buckets/warehouse/pedsnet"
const table = namespace + "/person"
var deleted []string
mock := &mockFilerOps{
countFn: func(_ util.FullPath) (int, error) {
return 0, nil
},
deleteFn: func(path util.FullPath) error {
deleted = append(deleted, string(path))
return nil
},
attrsFn: func(path util.FullPath) (map[string][]byte, error) {
if path == namespace || path == table {
return map[string][]byte{s3_constants.ExtS3TablesPrefix + "metadata": []byte("{}")}, nil
}
return nil, nil
},
}
cleaner := &EmptyFolderCleaner{
filer: mock,
lockRing: lockRing,
host: "filer1:8888",
bucketPath: "/buckets",
enabled: true,
folderCounts: make(map[string]*folderState),
cleanupQueue: NewCleanupQueue(1000, time.Minute),
maxCountCheck: 1000,
cacheExpiry: time.Minute,
processorSleep: time.Second,
stopCh: make(chan struct{}),
}
// The dropped table left its metadata folder empty; the cascade up from it must
// stop at the table the rename put back, and at the namespace above it.
cleaner.executeCleanup(table+"/metadata", "v2.metadata.json")
if len(deleted) != 1 || deleted[0] != table+"/metadata" {
t.Fatalf("expected only %s to be deleted, got %v", table+"/metadata", deleted)
}
}
func TestEmptyFolderCleaner_restoreIfWrittenTo(t *testing.T) { func TestEmptyFolderCleaner_restoreIfWrittenTo(t *testing.T) {
lockRing := lock_manager.NewLockRing(5 * time.Second) lockRing := lock_manager.NewLockRing(5 * time.Second)
lockRing.SetSnapshot([]pb.ServerAddress{"filer1:8888"}, 0) lockRing.SetSnapshot([]pb.ServerAddress{"filer1:8888"}, 0)
+4
View File
@@ -49,6 +49,10 @@ const (
// Bucket Policy // Bucket Policy
ExtBucketPolicyKey = "Seaweed-X-Amz-Bucket-Policy" ExtBucketPolicyKey = "Seaweed-X-Amz-Bucket-Policy"
// Every attribute the s3tables catalog stores on a table bucket, namespace,
// table or view directory entry starts with this.
ExtS3TablesPrefix = "s3tables."
// Object Retention and Legal Hold // Object Retention and Legal Hold
ExtObjectLockModeKey = "Seaweed-X-Amz-Object-Lock-Mode" ExtObjectLockModeKey = "Seaweed-X-Amz-Object-Lock-Mode"
ExtRetentionUntilDateKey = "Seaweed-X-Amz-Retention-Until-Date" ExtRetentionUntilDateKey = "Seaweed-X-Amz-Retention-Until-Date"
+8 -8
View File
@@ -21,17 +21,17 @@ const (
DefaultRegion = "us-east-1" DefaultRegion = "us-east-1"
// Extended entry attributes for metadata storage // Extended entry attributes for metadata storage
ExtendedKeyTableBucket = "s3tables.tableBucket" ExtendedKeyTableBucket = s3_constants.ExtS3TablesPrefix + "tableBucket"
ExtendedKeyMetadata = "s3tables.metadata" ExtendedKeyMetadata = s3_constants.ExtS3TablesPrefix + "metadata"
ExtendedKeyMetadataVersion = "s3tables.metadataVersion" ExtendedKeyMetadataVersion = s3_constants.ExtS3TablesPrefix + "metadataVersion"
ExtendedKeyPolicy = "s3tables.policy" ExtendedKeyPolicy = s3_constants.ExtS3TablesPrefix + "policy"
ExtendedKeyTags = "s3tables.tags" ExtendedKeyTags = s3_constants.ExtS3TablesPrefix + "tags"
ExtendedKeyMaintenance = "s3tables.maintenance" ExtendedKeyMaintenance = s3_constants.ExtS3TablesPrefix + "maintenance"
// Written by the maintenance worker, read by GetTableMaintenanceJobStatus. // Written by the maintenance worker, read by GetTableMaintenanceJobStatus.
// Separate from ExtendedKeyMaintenance so worker and operator writes do not // Separate from ExtendedKeyMaintenance so worker and operator writes do not
// contend on the same attribute. // contend on the same attribute.
ExtendedKeyMaintenanceStatus = "s3tables.maintenanceStatus" ExtendedKeyMaintenanceStatus = s3_constants.ExtS3TablesPrefix + "maintenanceStatus"
ExtendedKeyEntryType = "s3tables.entryType" ExtendedKeyEntryType = s3_constants.ExtS3TablesPrefix + "entryType"
// Entry-type marker values for ExtendedKeyEntryType. Absent or "table" means // Entry-type marker values for ExtendedKeyEntryType. Absent or "table" means
// a table; views are stored like tables but tagged "view". // a table; views are stored like tables but tagged "view".