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>
This commit is contained in:
authored and GitHub committed 2026-11-01 13:43:15 -08:00
1 parent 55f1e1efc5
commit d6ec6bc4e7
9 files changed
+1628 -67

No files matched your search

+85 -11
View File
@@ -170,20 +170,50 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
}
namespacePath := GetNamespacePath(bucketName, namespaceName)
// The authorization above holds for this bucket only. One deleted and
// created again under the same name since then is another bucket, and is
// treated as gone by both checks below.
isAuthorizedBucket := isTableBucketCreatedAt(bucketMetadata.CreatedAt)
// Check if namespace already exists
// A delete that has claimed the bucket is scanning it for namespaces, so
// refuse before writing anything. Checked after the authorization above, so
// denied callers never see the delete.
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
_, err := h.readNamespaceMetadataAttr(r.Context(), client, namespacePath)
return h.checkParentOpen(r.Context(), client, bucketPath, isAuthorizedBucket)
})
if h.writeBucketClosedError(w, err, bucketName) {
return err
}
if err != nil {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table bucket: %v", err))
return err
}
// Check if namespace already exists. One still marked unfinished does not:
// its create either confirmed it or lost it, and the mark left behind would
// wedge the name, so this create takes it over the way a table's does.
exists := false
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
entry, err := h.lookupEntry(r.Context(), client, namespacePath)
if errors.Is(err, filer_pb.ErrNotFound) {
return nil
}
if err != nil {
return err
}
_, hasMetadata := entry.Extended[ExtendedKeyMetadata]
exists = hasMetadata && !isUnfinishedCreate(entry.Extended)
return nil
})
if err == nil {
h.writeError(w, http.StatusConflict, ErrCodeNamespaceAlreadyExists, fmt.Sprintf("namespace %s already exists", namespaceName))
return fmt.Errorf("namespace already exists")
} else if !errors.Is(err, filer_pb.ErrNotFound) {
if err != nil {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check namespace: %v", err))
return err
}
if exists {
h.writeError(w, http.StatusConflict, ErrCodeNamespaceAlreadyExists, fmt.Sprintf("namespace %s already exists", namespaceName))
return fmt.Errorf("namespace already exists")
}
// Create the namespace with bucket owner to maintain consistency
// (authorization above ensures the caller has permission to create in this bucket)
@@ -201,7 +231,13 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
return fmt.Errorf("failed to marshal metadata: %w", err)
}
attrs := map[string][]byte{ExtendedKeyMetadata: metadataBytes}
// The unfinished mark keeps a table or view from being created in the
// namespace until the bucket check below has passed: one created before it
// would be left under a namespace that check then backs out.
attrs := map[string][]byte{
ExtendedKeyMetadata: metadataBytes,
ExtendedKeyDeleting: []byte(createClaim),
}
var taken error
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
err := h.createDirectoryExclusive(r.Context(), client, namespacePath, attrs)
@@ -215,7 +251,19 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
return nil
}
}
return err
if err != nil {
return err
}
// A delete may have claimed the bucket since the check above, after it
// scanned for namespaces and before this write. Confirm the bucket once
// more now that the namespace is there: a delete claiming it later scans
// the namespace, marked or not, and one that claimed it already makes
// this back out what it wrote. The mark comes off only after that check
// and the bucket is not checked again: a table created in the window
// between the two would be left under a namespace the second check backs out.
// A bucket recreated since the authorization fails the check as gone, so
// the namespace is backed out of it rather than left with this owner.
return h.confirmParentOnce(r.Context(), client, bucketPath, isAuthorizedBucket, namespacePath, attrs, namespacePath)
})
if err == nil && taken != nil {
err = taken
@@ -228,6 +276,9 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
h.writeError(w, http.StatusConflict, ErrCodeConflict, "namespace changed during create, retry the request")
return err
}
if h.writeBucketClosedError(w, err, bucketName) {
return err
}
if err != nil {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, "failed to create namespace")
return err
@@ -243,6 +294,25 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
return nil
}
// writeBucketClosedError answers a create refused because its bucket is being
// created, being deleted, or was deleted, and reports whether err was one of
// those.
func (h *S3TablesHandler) writeBucketClosedError(w http.ResponseWriter, err error, bucketName string) bool {
switch {
case errors.Is(err, errParentCreating):
h.writeError(w, http.StatusConflict, ErrCodeConflict, fmt.Sprintf("table bucket %s is still being created, retry the request", bucketName))
case errors.Is(err, errParentDeleting):
h.writeError(w, http.StatusConflict, ErrCodeConflict, fmt.Sprintf("table bucket %s is being deleted", bucketName))
case errors.Is(err, errParentGone):
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
case errors.Is(err, errBackOutFailed):
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("table bucket %s was deleted during the request and the new namespace may remain", bucketName))
default:
return false
}
return true
}
// handleGetNamespace gets details of a namespace
func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
var req GetNamespaceRequest
@@ -580,9 +650,10 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re
continue
}
// Read metadata from extended attribute
// Read metadata from extended attribute. A namespace its create has
// not confirmed is not one yet.
data, ok := entry.Entry.Extended[ExtendedKeyMetadata]
if !ok {
if !ok || isUnfinishedCreate(entry.Entry.Extended) {
continue
}
@@ -718,11 +789,14 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R
}
// Claim before scanning, so a create that writes after the scan sees it.
// A namespace whose create has not confirmed it is absent, and claiming it
// would replace the create's mark: a refused delete then lifts the claim and
// leaves the namespace looking confirmed before its create checked the bucket.
var claim string
var claimed map[string][]byte
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
var claimErr error
claim, claimed, claimErr = h.claimForDelete(r.Context(), client, namespacePath, isNamespaceEntry)
claim, claimed, claimErr = h.claimForDelete(r.Context(), client, namespacePath, isFinishedNamespaceEntry)
return claimErr
})
if err != nil {