From ab4e52ae2ff268759bc28982f5f8e91d8a7158d3 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Apr 2026 11:55:13 -0700 Subject: [PATCH] fix(s3): use recursive delete for .versions directory cleanup (#8887) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(s3): use recursive delete for .versions directory cleanup When only delete markers remain in a .versions directory, updateLatestVersionAfterDeletion tried to delete it non-recursively, which failed with "fail to delete non-empty folder" because the delete marker entries were still present. Use recursive deletion so the directory and its remaining delete marker entries are cleaned up together. * fix(s3): guard .versions directory deletion against truncated listings When the version listing is truncated (>1000 entries), content versions may exist beyond the first page. Skip the recursive directory deletion in this case to prevent data loss. * fix(s3): preserve delete markers in .versions directory Delete markers must be preserved per S3 semantics — they are only removed by an explicit DELETE with versionId. The previous fix would recursively delete the entire .versions directory (including delete markers) when no content versions were found. Now the logic distinguishes three cases: 1. Content versions exist → update latest version metadata 2. Only delete markers remain (or listing truncated) → keep directory 3. Truly empty → safe to delete directory (non-recursive) --- weed/s3api/s3api_object_versioning.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index 4ffb3970f..07f3cc250 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -999,7 +999,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) glog.V(1).Infof("updateLatestVersionAfterDeletion: updating latest version for %s/%s, listing %s", bucket, object, versionsDir) // List all remaining version files in the .versions directory - entries, _, err := s3a.list(versionsDir, "", "", false, 1000) + entries, isLast, err := s3a.list(versionsDir, "", "", false, 1000) if err != nil { glog.Errorf("updateLatestVersionAfterDeletion: failed to list versions in %s: %v", versionsDir, err) return fmt.Errorf("failed to list versions: %v", err) @@ -1011,6 +1011,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) var latestVersionId string var latestVersionFileName string var latestVersionEntry *filer_pb.Entry + hasDeleteMarkers := false for _, entry := range entries { if entry.Extended == nil { @@ -1027,6 +1028,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) // Skip delete markers when finding latest content version isDeleteMarkerBytes, _ := entry.Extended[s3_constants.ExtDeleteMarkerKey] if string(isDeleteMarkerBytes) == "true" { + hasDeleteMarkers = true continue } @@ -1070,14 +1072,18 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) if err != nil { return fmt.Errorf("failed to update .versions directory metadata: %v", err) } + } else if hasDeleteMarkers || !isLast { + // Delete markers still exist in the .versions directory, or the listing was + // truncated so there may be more entries. Either way, keep the directory. + glog.V(2).Infof("updateLatestVersionAfterDeletion: no content versions found for %s/%s but .versions directory still has entries (deleteMarkers=%v, isLast=%v), keeping directory", + bucket, object, hasDeleteMarkers, isLast) } else { - // No versions left - delete the .versions metadata file entirely - // This prevents clients from seeing an empty .versions file - glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions metadata file", bucket, object) + // No entries at all - delete the .versions directory entirely + glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions directory", bucket, object) err = s3a.rm(bucketDir, versionsObjectPath, true, false) if err != nil { - glog.Warningf("updateLatestVersionAfterDeletion: failed to delete .versions metadata file for %s/%s: %v", bucket, object, err) + glog.Warningf("updateLatestVersionAfterDeletion: failed to delete .versions directory for %s/%s: %v", bucket, object, err) // Don't return error - the versions are already deleted, this is just cleanup } }