From ff42c97ab72dd01a4ae60727b0cca027b7637927 Mon Sep 17 00:00:00 2001
From: Claudio Costa <cstcld91@gmail.com>
Date: Fri, 2 Oct 2020 19:14:16 +0200
Subject: [PATCH] [MM-29229] Set part size for S3 uploads to avoid excessive
 allocation (#15737)

* Set part size for S3 uploads

* Include benchmark
---
 services/filesstore/filesstore_test.go | 37 ++++++++++++++++++++++++++
 services/filesstore/s3store.go         | 14 ++++------
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/services/filesstore/filesstore_test.go b/services/filesstore/filesstore_test.go
index a109d7d892..3bb5f2298f 100644
--- a/services/filesstore/filesstore_test.go
+++ b/services/filesstore/filesstore_test.go
@@ -364,3 +364,40 @@ func (s *FileBackendTestSuite) TestAppendFile() {
 		s.EqualValues(append(append(b, b2...), b3...), read)
 	})
 }
+
+func BenchmarkS3WriteFile(b *testing.B) {
+	utils.TranslationsPreInit()
+
+	settings := &model.FileSettings{
+		DriverName:              model.NewString(model.IMAGE_DRIVER_S3),
+		AmazonS3AccessKeyId:     model.NewString(model.MINIO_ACCESS_KEY),
+		AmazonS3SecretAccessKey: model.NewString(model.MINIO_SECRET_KEY),
+		AmazonS3Bucket:          model.NewString(model.MINIO_BUCKET),
+		AmazonS3Region:          model.NewString(""),
+		AmazonS3Endpoint:        model.NewString("localhost:9000"),
+		AmazonS3PathPrefix:      model.NewString(""),
+		AmazonS3SSL:             model.NewBool(false),
+		AmazonS3SSE:             model.NewBool(false),
+	}
+
+	backend, err := NewFileBackend(settings, true)
+	require.Nil(b, err)
+
+	// This is needed to create the bucket if it doesn't exist.
+	require.Nil(b, backend.TestConnection())
+
+	path := "tests/" + model.NewId()
+	size := 1 * 1024 * 1024
+	data := make([]byte, size)
+
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		written, err := backend.WriteFile(bytes.NewReader(data), path)
+		defer backend.RemoveFile(path)
+		require.Nil(b, err)
+		require.Equal(b, len(data), int(written))
+	}
+
+	b.StopTimer()
+}
diff --git a/services/filesstore/s3store.go b/services/filesstore/s3store.go
index 465973adfe..4f12006554 100644
--- a/services/filesstore/s3store.go
+++ b/services/filesstore/s3store.go
@@ -264,15 +264,8 @@ func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, *model.App
 		contentType = "binary/octet-stream"
 	}
 
-	var sse encrypt.ServerSide
-	if b.encrypt {
-		sse = encrypt.NewSSE()
-	}
-	options := s3.PutObjectOptions{
-		ContentType:          contentType,
-		ServerSideEncryption: sse,
-	}
-
+	options := s3PutOptions(b.encrypt, contentType)
+	sse := options.ServerSideEncryption
 	partName := fp + ".part"
 	info, err := s3Clnt.PutObject(context.Background(), b.bucket, partName, fr, -1, options)
 	defer s3Clnt.RemoveObject(context.Background(), b.bucket, partName, s3.RemoveObjectOptions{})
@@ -393,6 +386,9 @@ func s3PutOptions(encrypted bool, contentType string) s3.PutObjectOptions {
 		options.ServerSideEncryption = encrypt.NewSSE()
 	}
 	options.ContentType = contentType
+	// We set the part size to the minimum allowed value of 5MBs
+	// to avoid an excessive allocation in minio.PutObject implementation.
+	options.PartSize = 1024 * 1024 * 5
 
 	return options
 }