From e3fe02f87dae2d18e2c7c52416f32be8b725e583 Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 10:35:59 +0800 Subject: [PATCH 1/7] pkg/gateway: fix ListMultipartUploads sorting rule and support delimiter --- pkg/gateway/gateway.go | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 5cb4019bc2b9..5b646abefa2b 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path" + "slices" "sort" "strconv" "strings" @@ -842,12 +843,14 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr lmi.KeyMarker = keyMarker lmi.UploadIDMarker = uploadIDMarker lmi.MaxUploads = maxUploads + lmi.Delimiter = delimiter + commPrefixSet := make(map[string]struct{}) for _, e := range entries { uploadID := string(e.Name) - if uploadID > uploadIDMarker { - object_, _ := n.fs.GetXattr(mctx, n.upath(bucket, uploadID), uploadKeyName) - object := string(object_) - if strings.HasPrefix(object, prefix) && object > keyMarker { + object_, _ := n.fs.GetXattr(mctx, n.upath(bucket, uploadID), uploadKeyName) + object := string(object_) + if strings.HasPrefix(object, prefix) { + if keyMarker != "" && object+uploadID > keyMarker+uploadIDMarker || keyMarker == "" && object > keyMarker { lmi.Uploads = append(lmi.Uploads, minio.MultipartInfo{ Object: object, UploadID: uploadID, @@ -856,11 +859,34 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr } } } + + slices.SortFunc(lmi.Uploads, func(a, b minio.MultipartInfo) int { + return strings.Compare(a.Object+a.UploadID, b.Object+b.UploadID) + }) + if len(lmi.Uploads) > maxUploads { lmi.IsTruncated = true lmi.Uploads = lmi.Uploads[:maxUploads] - lmi.NextKeyMarker = keyMarker - lmi.NextUploadIDMarker = lmi.Uploads[maxUploads-1].UploadID + } + + if delimiter != "" { + for i := len(lmi.Uploads) - 1; i >= 0; i-- { + index := strings.Index(strings.TrimLeft(lmi.Uploads[i].Object, prefix), delimiter) + if index != -1 { + commPrefixSet[lmi.Uploads[i].Object[:index+1]] = struct{}{} + lmi.Uploads = append(lmi.Uploads[:i], lmi.Uploads[i+1:]...) + continue + } + } + for prefix := range commPrefixSet { + lmi.CommonPrefixes = append(lmi.CommonPrefixes, prefix) + } + slices.Sort(lmi.CommonPrefixes) + } + + if len(lmi.Uploads) != 0 { + lmi.NextKeyMarker = lmi.Uploads[len(lmi.Uploads)-1].Object + lmi.NextUploadIDMarker = lmi.Uploads[len(lmi.Uploads)-1].UploadID } return lmi, jfsToObjectErr(ctx, err, bucket) } From b597499fb176c7d1b5d34dd862d1789cab078c32 Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 10:41:54 +0800 Subject: [PATCH 2/7] pkg/gateway: fix build --- pkg/gateway/gateway.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 5b646abefa2b..21b31564459f 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -23,7 +23,6 @@ import ( "net/http" "os" "path" - "slices" "sort" "strconv" "strings" @@ -860,8 +859,8 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr } } - slices.SortFunc(lmi.Uploads, func(a, b minio.MultipartInfo) int { - return strings.Compare(a.Object+a.UploadID, b.Object+b.UploadID) + sort.Slice(lmi.Uploads, func(i, j int) bool { + return lmi.Uploads[i].Object+lmi.Uploads[i].UploadID < lmi.Uploads[j].Object+lmi.Uploads[j].UploadID }) if len(lmi.Uploads) > maxUploads { @@ -881,7 +880,7 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr for prefix := range commPrefixSet { lmi.CommonPrefixes = append(lmi.CommonPrefixes, prefix) } - slices.Sort(lmi.CommonPrefixes) + sort.Strings(lmi.CommonPrefixes) } if len(lmi.Uploads) != 0 { From d2badf39e592489b3f5c0a37e4b155b698a2d41c Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 13:42:32 +0800 Subject: [PATCH 3/7] fix max-upload --- pkg/gateway/gateway.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 21b31564459f..2346655e18d2 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -846,10 +846,11 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr commPrefixSet := make(map[string]struct{}) for _, e := range entries { uploadID := string(e.Name) + // todo: parallel object_, _ := n.fs.GetXattr(mctx, n.upath(bucket, uploadID), uploadKeyName) object := string(object_) if strings.HasPrefix(object, prefix) { - if keyMarker != "" && object+uploadID > keyMarker+uploadIDMarker || keyMarker == "" && object > keyMarker { + if keyMarker != "" && object+uploadID > keyMarker+uploadIDMarker || keyMarker == "" { lmi.Uploads = append(lmi.Uploads, minio.MultipartInfo{ Object: object, UploadID: uploadID, @@ -863,24 +864,34 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr return lmi.Uploads[i].Object+lmi.Uploads[i].UploadID < lmi.Uploads[j].Object+lmi.Uploads[j].UploadID }) - if len(lmi.Uploads) > maxUploads { - lmi.IsTruncated = true - lmi.Uploads = lmi.Uploads[:maxUploads] - } - if delimiter != "" { - for i := len(lmi.Uploads) - 1; i >= 0; i-- { - index := strings.Index(strings.TrimLeft(lmi.Uploads[i].Object, prefix), delimiter) - if index != -1 { - commPrefixSet[lmi.Uploads[i].Object[:index+1]] = struct{}{} - lmi.Uploads = append(lmi.Uploads[:i], lmi.Uploads[i+1:]...) - continue + var tmp []minio.MultipartInfo + for _, info := range lmi.Uploads { + if maxUploads == 0 { + lmi.IsTruncated = true + break + } + index := strings.Index(strings.TrimLeft(info.Object, prefix), delimiter) + if index == -1 { + tmp = append(tmp, info) + maxUploads-- + } else { + commPrefix := info.Object[:index+1] + if _, ok := commPrefixSet[commPrefix]; ok { + continue + } + commPrefixSet[commPrefix] = struct{}{} + maxUploads-- } } + lmi.Uploads = tmp for prefix := range commPrefixSet { lmi.CommonPrefixes = append(lmi.CommonPrefixes, prefix) } sort.Strings(lmi.CommonPrefixes) + } else if len(lmi.Uploads) > maxUploads { + lmi.IsTruncated = true + lmi.Uploads = lmi.Uploads[:maxUploads] } if len(lmi.Uploads) != 0 { From 17886829ace9821ec9425691474dac11ae1bcfcc Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 17:17:56 +0800 Subject: [PATCH 4/7] add test --- integration/s3gateway_test.sh | 72 +++++++++++++++++++++++++++++++++++ pkg/gateway/gateway.go | 6 ++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/integration/s3gateway_test.sh b/integration/s3gateway_test.sh index b4c5b0a10aa8..601221071ab5 100755 --- a/integration/s3gateway_test.sh +++ b/integration/s3gateway_test.sh @@ -732,6 +732,78 @@ function test_multipart_upload() { fi fi + + for key in "afile" "bfile" "bfile" "documents/report1.pdf" "documents/report2.pdf" "ebook" "photos/2021/a2.png" "photos/2021/a3.png" "photos/2022/a4.png" + do + if [ $rv -eq 0 ]; then + # create multipart + function="${AWS} s3api create-multipart-upload --bucket ${bucket_name} --key ${key}" + test_function=${function} + out=$($function) + rv=$? + upload_id=$(echo "$out" | jq -r .UploadId) + fi + done + + if [ $rv -eq 0 ]; then + function="${AWS} s3api list-multipart-uploads --bucket ${bucket_name}" + test_function=${function} + out=$($function) + rv=$? + keys=$(echo "$out" | jq 'foreach .Uploads[] as $upload (null; . + $upload.Key)') + if [ $keys != "afilebfilebfiledocuments/report1.pdfdocuments/report2.pdfebookphotos/2021/a2.pngphotos/2021/a3.pngphotos/2022/a4.png" ]; then + rv=1 + out="list-multipart-uploads failed" + fi + fi + + if [ $rv -eq 0 ]; then + function="${AWS} s3api list-multipart-uploads --bucket ${bucket_name} --key-marker bfile" + test_function=${function} + out=$($function) + rv=$? + keys=$(echo "$out" | jq 'foreach .Uploads[] as $upload (null; . + $upload.Key)') + if [ $keys != "bfilebfiledocuments/report1.pdfdocuments/report2.pdfebookphotos/2021/a2.pngphotos/2021/a3.pngphotos/2022/a4.png" ]; then + rv=1 + out="list-multipart-upload failed" + fi + fi + + if [ $rv -eq 0 ]; then + function="${AWS} s3api list-multipart-uploads --bucket ${bucket_name} --key-marker bfile --delimiter /" + test_function=${function} + out=$($function) + rv=$? + keys=$(echo "$out" | jq 'foreach .Uploads[] as $upload (null; . + $upload.Key)') + if [ $keys != "afilebfilebfileebook" ]; then + rv=1 + out="list-multipart-uploads failed" + fi + keys=$(echo "$out" | jq 'foreach .CommonPrefixes[] as $CommonPrefix (null; . + $CommonPrefix.Prefix)') + if [ $keys != "documents/photos/" ]; then + rv=1 + out="list-multipart-uploads failed" + fi + fi + + if [ $rv -eq 0 ]; then + function="${AWS} s3api list-multipart-uploads --bucket ${bucket_name} --delimiter / --max-upload 5" + test_function=${function} + out=$($function) + rv=$? + keys=$(echo "$out" | jq 'foreach .Uploads[] as $upload (null; . + $upload.Key)') + if [ $keys != "afilebfilebfileebook" ]; then + rv=1 + out="list-multipart-uploads failed" + fi + keys=$(echo "$out" | jq -r '.CommonPrefixes[0].Prefix') + if [ $keys != "documents/" ]; then + rv=1 + out="list-multipart-uploads failed" + fi + fi + + if [ $rv -eq 0 ]; then function="delete_bucket" out=$(delete_bucket "$bucket_name") diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 2346655e18d2..3470a8f39c46 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -847,7 +847,11 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr for _, e := range entries { uploadID := string(e.Name) // todo: parallel - object_, _ := n.fs.GetXattr(mctx, n.upath(bucket, uploadID), uploadKeyName) + object_, eno := n.fs.GetXattr(mctx, n.upath(bucket, uploadID), uploadKeyName) + if eno != 0 { + logger.Warnf("get object xattr error %s: %s, ignore this item", n.upath(bucket, uploadID), eno) + continue + } object := string(object_) if strings.HasPrefix(object, prefix) { if keyMarker != "" && object+uploadID > keyMarker+uploadIDMarker || keyMarker == "" { From e52382ca7d59c5805b444c45558a8a66d3b91e3d Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 17:43:42 +0800 Subject: [PATCH 5/7] fix sort --- pkg/gateway/gateway.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 3470a8f39c46..5d9ff6f45e75 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -865,7 +865,11 @@ func (n *jfsObjects) ListMultipartUploads(ctx context.Context, bucket string, pr } sort.Slice(lmi.Uploads, func(i, j int) bool { - return lmi.Uploads[i].Object+lmi.Uploads[i].UploadID < lmi.Uploads[j].Object+lmi.Uploads[j].UploadID + if lmi.Uploads[i].Object == lmi.Uploads[j].Object { + return lmi.Uploads[i].UploadID < lmi.Uploads[j].UploadID + } else { + return lmi.Uploads[i].Object < lmi.Uploads[j].Object + } }) if delimiter != "" { From ab28dd57f183337c8bfae458065cf85736196094 Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 18:02:19 +0800 Subject: [PATCH 6/7] improve test --- integration/s3gateway_test.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/integration/s3gateway_test.sh b/integration/s3gateway_test.sh index 601221071ab5..752d21b11d79 100755 --- a/integration/s3gateway_test.sh +++ b/integration/s3gateway_test.sh @@ -803,6 +803,18 @@ function test_multipart_upload() { fi fi + if [ $rv -eq 0 ]; then + function="${AWS} s3api list-multipart-uploads --bucket ${bucket_name} --prefix documents/" + test_function=${function} + out=$($function) + rv=$? + keys=$(echo "$out" | jq 'foreach .Uploads[] as $upload (null; . + $upload.Key)') + if [ $keys != "documents/report1.pdfdocuments/report2.pdf" ]; then + rv=1 + out="list-multipart-upload failed" + fi + fi + fi if [ $rv -eq 0 ]; then function="delete_bucket" From 74d4021a206f9259ee8cb8275183e8f95fd212f3 Mon Sep 17 00:00:00 2001 From: zhijian Date: Wed, 27 Dec 2023 19:19:49 +0800 Subject: [PATCH 7/7] fix --- integration/s3gateway_test.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/s3gateway_test.sh b/integration/s3gateway_test.sh index 752d21b11d79..514dff49c778 100755 --- a/integration/s3gateway_test.sh +++ b/integration/s3gateway_test.sh @@ -813,8 +813,7 @@ function test_multipart_upload() { rv=1 out="list-multipart-upload failed" fi - fi - fi + fi if [ $rv -eq 0 ]; then function="delete_bucket"