fix(s3): use recursive delete for .versions directory cleanup (#8887)
* 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)
This commit is contained in:
@@ -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)
|
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
|
// 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 {
|
if err != nil {
|
||||||
glog.Errorf("updateLatestVersionAfterDeletion: failed to list versions in %s: %v", versionsDir, err)
|
glog.Errorf("updateLatestVersionAfterDeletion: failed to list versions in %s: %v", versionsDir, err)
|
||||||
return fmt.Errorf("failed to list versions: %v", 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 latestVersionId string
|
||||||
var latestVersionFileName string
|
var latestVersionFileName string
|
||||||
var latestVersionEntry *filer_pb.Entry
|
var latestVersionEntry *filer_pb.Entry
|
||||||
|
hasDeleteMarkers := false
|
||||||
|
|
||||||
for _, entry := range entries {
|
for _, entry := range entries {
|
||||||
if entry.Extended == nil {
|
if entry.Extended == nil {
|
||||||
@@ -1027,6 +1028,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string)
|
|||||||
// Skip delete markers when finding latest content version
|
// Skip delete markers when finding latest content version
|
||||||
isDeleteMarkerBytes, _ := entry.Extended[s3_constants.ExtDeleteMarkerKey]
|
isDeleteMarkerBytes, _ := entry.Extended[s3_constants.ExtDeleteMarkerKey]
|
||||||
if string(isDeleteMarkerBytes) == "true" {
|
if string(isDeleteMarkerBytes) == "true" {
|
||||||
|
hasDeleteMarkers = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1070,14 +1072,18 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string)
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to update .versions directory metadata: %v", err)
|
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 {
|
} else {
|
||||||
// No versions left - delete the .versions metadata file entirely
|
// No entries at all - delete the .versions directory entirely
|
||||||
// This prevents clients from seeing an empty .versions file
|
glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions directory", bucket, object)
|
||||||
glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions metadata file", bucket, object)
|
|
||||||
|
|
||||||
err = s3a.rm(bucketDir, versionsObjectPath, true, false)
|
err = s3a.rm(bucketDir, versionsObjectPath, true, false)
|
||||||
if err != nil {
|
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
|
// Don't return error - the versions are already deleted, this is just cleanup
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user