From f059acc7ccc443405cf9d465f619319483ed947a Mon Sep 17 00:00:00 2001 From: Markus Kienast Date: Thu, 24 Aug 2017 01:29:26 +0200 Subject: [PATCH] fixed edge cases; pre-cleanup --- s3store/s3store.go | 28 ++++++---- s3store/s3store_test.go | 118 +++++++++++++++++++++++++--------------- 2 files changed, 90 insertions(+), 56 deletions(-) diff --git a/s3store/s3store.go b/s3store/s3store.go index 86ac863..a925b70 100644 --- a/s3store/s3store.go +++ b/s3store/s3store.go @@ -565,27 +565,33 @@ func (store S3Store) CalcOptimalPartSize(size int64) (int64, error) { switch { // We can only manage files up to MaxObjectSize, else we need to fail. case size > store.MaxObjectSize: + fmt.Print("0 ") return 0, fmt.Errorf("CalcOptimalPartSize: size of %v bytes exceeds MaxObjectSize of %v bytes", size, store.MaxObjectSize) // When upload is smaller or equal MinPartSize, we upload in just one part. case size <= store.MinPartSize: + fmt.Print("A ") return store.MinPartSize, nil // When we need 9999 parts or less with MinPartSize. case size/store.MinPartSize < store.MaxMultipartParts: + fmt.Print("B ") return store.MinPartSize, nil - // When we can divide our upload into exactly MaxMultipartParts parts with - // no bytes leftover, we will not need an spare last part. - // Also, when MaxObjectSize is equal to MaxPartSize * MaxMultipartParts + // If our upload divides up exactly into MaxMultipartParts parts with + // no bytes leftover, we will not need an spare last part. So we can go with + // a straight division. Otherwise we would exceed MaxPartSize in a scenario, + // where MaxObjectSize is equal to MaxPartSize * MaxMultipartParts // (which is not the case with the current AWS S3 API specification, but - // might be in the future or with other S3-aware stores), we need this in - // order for our Multipart-Upload to reach full MaxObjectSize. + // might be in the future or with other S3-aware stores). case size%store.MaxMultipartParts == 0: + fmt.Print("C ") return size / store.MaxMultipartParts, nil - // If the last part would be larger than MaxPartSize, which is only the case - // when we are close to MaxObjectSize, we have to go with MaxPartSize. - case size%(store.MaxMultipartParts-1) > store.MaxPartSize: - return store.MaxPartSize, nil - // In all other cases, we need a spare last piece for the remaining bytes. + // In all other cases, we need to round up to the next integer, as long as we + // stay below MaxPartSize. + case size/store.MaxMultipartParts < store.MaxPartSize: + fmt.Print("D ") + return size/store.MaxMultipartParts + 1, nil + // If non of the above matches, we have exceeded the MaxPartSize limit above. default: - return size / (store.MaxMultipartParts - 1), nil + fmt.Print("X ") + return size/store.MaxMultipartParts + 1, fmt.Errorf("CalcOptimalPartSize: to upload %v bytes optimalPartSize %v must exceed MaxPartSize %v", size, (size/store.MaxMultipartParts + 1), store.MaxPartSize) } } diff --git a/s3store/s3store_test.go b/s3store/s3store_test.go index c640984..d5fdeef 100644 --- a/s3store/s3store_test.go +++ b/s3store/s3store_test.go @@ -3,11 +3,10 @@ package s3store_test import ( "bytes" "fmt" - "io/ioutil" - "testing" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "io/ioutil" + "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -36,23 +35,32 @@ func TestCalcOptimalPartSize(t *testing.T) { assert.Equal("bucket", store.Bucket) assert.Equal(s3obj, store.Service) + // If you quickly want to override the default values in this test + store.MinPartSize = 2 + store.MaxPartSize = 10 + store.MaxMultipartParts = 20 + store.MaxObjectSize = 201 + var MinPartSize = store.MinPartSize var MaxPartSize = store.MaxPartSize var MaxMultipartParts = store.MaxMultipartParts var MaxObjectSize = store.MaxObjectSize - // sanity check - if MaxObjectSize > MaxPartSize*MaxMultipartParts { - t.Errorf("%v parts with %v bytes each is %v bytes, which is less than MaxObjectSize=%v.\n", MaxMultipartParts, MaxPartSize, MaxMultipartParts*MaxPartSize, MaxObjectSize) - } - - var LimitedMaxPartSize = MaxObjectSize / (MaxMultipartParts - 1) - // the size of the last part, when upload has MaxObjectSize and we use - // LimitedMaxPartSize for uploading - var LastPartSize int64 = MaxObjectSize % (MaxMultipartParts - 1) - var equalparts, lastpartsize int64 var err string + // sanity check + if MaxObjectSize > MaxPartSize*MaxMultipartParts { + t.Errorf("MaxObjectSize %v can never be achieved, as MaxMultipartParts %v and MaxPartSize %v only allow for an upload of %v bytes total.\n", MaxObjectSize, MaxMultipartParts, MaxPartSize, MaxMultipartParts*MaxPartSize) + } + var HighestApplicablePartSize int64 = MaxObjectSize / MaxMultipartParts + if MaxObjectSize%MaxMultipartParts > 0 { + HighestApplicablePartSize++ + } + var RemainderWithHighestApplicablePartSize int64 = MaxObjectSize % HighestApplicablePartSize + + fmt.Println("HighestApplicablePartSize", HighestApplicablePartSize) + fmt.Println("RemainderWithHighestApplicablePartSize", RemainderWithHighestApplicablePartSize) + // some of these tests are actually duplicates, as they specify the same size // in bytes - two ways to describe the same thing. That is wanted, in order // to provide a full picture from any angle. @@ -61,58 +69,78 @@ func TestCalcOptimalPartSize(t *testing.T) { MinPartSize - 1, MinPartSize, MinPartSize + 1, - MinPartSize * 9999, - MinPartSize*10000 - 1, - MinPartSize * 10000, - MinPartSize*10000 + 1, - MinPartSize * 10001, - LimitedMaxPartSize*9999 - 1, - LimitedMaxPartSize * 9999, - LimitedMaxPartSize*9999 + 1, - LimitedMaxPartSize*9999 + LastPartSize - 1, - LimitedMaxPartSize*9999 + LastPartSize, - LimitedMaxPartSize*9999 + LastPartSize + 1, + + MinPartSize*(MaxMultipartParts-1) - 1, + MinPartSize * (MaxMultipartParts - 1), + MinPartSize*(MaxMultipartParts-1) + 1, + + MinPartSize*MaxMultipartParts - 1, + MinPartSize * MaxMultipartParts, + MinPartSize*MaxMultipartParts + 1, + + MinPartSize*(MaxMultipartParts+1) - 1, + MinPartSize * (MaxMultipartParts + 1), + MinPartSize*(MaxMultipartParts+1) + 1, + + (HighestApplicablePartSize-1)*MaxMultipartParts - 1, + (HighestApplicablePartSize - 1) * MaxMultipartParts, + (HighestApplicablePartSize-1)*MaxMultipartParts + 1, + + HighestApplicablePartSize*(MaxMultipartParts-1) - 1, + HighestApplicablePartSize * (MaxMultipartParts - 1), + HighestApplicablePartSize*(MaxMultipartParts-1) + 1, + + HighestApplicablePartSize*(MaxMultipartParts-1) + RemainderWithHighestApplicablePartSize - 1, + HighestApplicablePartSize*(MaxMultipartParts-1) + RemainderWithHighestApplicablePartSize, + HighestApplicablePartSize*(MaxMultipartParts-1) + RemainderWithHighestApplicablePartSize + 1, + MaxObjectSize - 1, MaxObjectSize, MaxObjectSize + 1, - MaxPartSize*9999 - 1, - MaxPartSize * 9999, - MaxPartSize*9999 + 1, - MaxPartSize*10000 - 1, - MaxPartSize * 10000, - MaxPartSize*10000 + 1, + + (MaxObjectSize/MaxMultipartParts)*(MaxMultipartParts-1) - 1, + (MaxObjectSize / MaxMultipartParts) * (MaxMultipartParts - 1), + (MaxObjectSize/MaxMultipartParts)*(MaxMultipartParts-1) + 1, + + MaxPartSize*(MaxMultipartParts-1) - 1, + MaxPartSize * (MaxMultipartParts - 1), + MaxPartSize*(MaxMultipartParts-1) + 1, + + MaxPartSize*MaxMultipartParts - 1, + MaxPartSize * MaxMultipartParts, + MaxPartSize*MaxMultipartParts + 1, } for index, size := range testcases { optimalPartSize, calcError := store.CalcOptimalPartSize(size) + equalparts, lastpartsize = 0, 0 + err = "" if size > MaxObjectSize && calcError == nil { - err += fmt.Sprintf("Testcase #%v: size=%v exceeds MaxObjectSize=%v but CalcOptimalPartSize did not throw an error\n", index, size, MaxObjectSize) - } - if optimalPartSize*(MaxMultipartParts-1) > MaxObjectSize { - err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v, exceeds MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) - } - if optimalPartSize > MaxPartSize { - err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v exceeds MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) + err += fmt.Sprintf("Testcase #%v size %v: size exceeds MaxObjectSize=%v but no error returned\n", index, size, MaxObjectSize) } if calcError == nil { equalparts = size / optimalPartSize lastpartsize = size % optimalPartSize if optimalPartSize < MinPartSize { - err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v is below MinPartSize=%v\n", index, optimalPartSize, MinPartSize) + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: optimalPartSize < MinPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MinPartSize) } if optimalPartSize > MaxPartSize { - err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v is larger than MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: optimalPartSize > MaxPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MaxPartSize) + } + if size%optimalPartSize == 0 && equalparts > MaxMultipartParts { + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: more parts than MaxMultipartParts %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MaxMultipartParts) + } + if size%optimalPartSize > 0 && equalparts > MaxMultipartParts-1 { + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: more parts than MaxMultipartParts %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MaxMultipartParts) } if lastpartsize > MaxPartSize { - err += fmt.Sprintf("Testcase #%v: size of last part %v is exceeding MaxPartSize limit of %v\n", index, lastpartsize, MaxPartSize) + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: lastpart > MaxPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MaxPartSize) } - if equalparts > 10000 { - err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts. %v equal parts of size %v, size of last part %v\n", index, equalparts, equalparts, optimalPartSize, lastpartsize) - } - if equalparts == 10000 && lastpartsize > 0 { - err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts. %v equal parts of size %v, size of last part %v\n", index, equalparts+1, equalparts, optimalPartSize, lastpartsize) + if lastpartsize > optimalPartSize { + err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: lastpart > optimalPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, optimalPartSize) } } + fmt.Printf("Testcase #%v size %v, %v parts of size %v, lastpart %v\n", index, size, equalparts, optimalPartSize, lastpartsize) if len(err) > 0 { t.Errorf(err) }