New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add S3FileSystem Write support #6021

Closed

majetideepak wants to merge 3 commits into facebookincubator:main from majetideepak:support-s3-write

Collaborator

majetideepak commented Aug 7, 2023 •

edited

S3WriteFile uses the Apache Arrow implementation as a reference.
AWS C++ SDK allows streaming writes via the MultiPart upload API.
Multipart upload allows you to upload a single object as a set of parts.
Each part is a contiguous portion of the object's data.
While AWS and Minio support different sizes for each
part (only requiring a minimum of 5MB), Certain object stores require that every
part be exactly equal (except for the last part). We set this to 10 MiB, so
that in combination with the maximum number of parts of 10,000, this gives a
file limit of 100k MiB (or about 98 GiB).
You can upload these object parts independently and in any order.
After all parts of your object are uploaded, Amazon S3 assembles these parts
and creates the object.
S3WriteFile is not thread-safe.
UploadPart is currently synchronous during append. Flush is no-op as append
handles all the uploads.

Resolves: #4805

netlify bot commented Aug 7, 2023 •

edited

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`29769d5`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/650463001592f00008daf269

facebook-github-bot added the CLA Signed label

Collaborator Author

majetideepak commented Aug 7, 2023 •

edited

CC: @akashsha1, @paul-amonson, @tigrux for early feedback on the design.
I need to run some performance tests to wrap up this. I will share them.

paul-amonson reviewed

View reviewed changes

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated Show resolved Hide resolved

paul-amonson reviewed

View reviewed changes

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated Show resolved Hide resolved

akashsha1 reviewed

View reviewed changes

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated

+                    request.SetBucket(awsString(bucket_));
+                    request.SetKey(awsString(key_));
+                    auto objectMetadata = client_->HeadObject(request);
+                    VELOX_CHECK(!objectMetadata.IsSuccess(), "File already exists");

Contributor

akashsha1 Aug 7, 2023

should this error message be "S3 object already exists"?

Collaborator Author

majetideepak Aug 7, 2023

Will Fix.

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated Show resolved Hide resolved

velox/connectors/hive/storage_adapters/s3fs/tests/S3FileSystemTest.cpp Show resolved Hide resolved

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Show resolved Hide resolved

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated Show resolved Hide resolved

tigrux reviewed

View reviewed changes

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated Show resolved Hide resolved

majetideepak force-pushed the support-s3-write branch from 7ca0135 to 1c9f36b Compare

August 25, 2023 11:00

This was referenced Aug 28, 2023

Add S3 Filesink #6309

Closed

Add write support in S3FileSystem #4805

Closed

majetideepak force-pushed the support-s3-write branch 2 times, most recently from 97f2109 to 5b14e7e Compare

September 12, 2023 02:58

majetideepak marked this pull request as ready for review

September 12, 2023 02:59

tigrux approved these changes

View reviewed changes

Contributor

tigrux left a comment

Very clean code.

paul-amonson approved these changes

View reviewed changes

Contributor

paul-amonson left a comment

:)

majetideepak force-pushed the support-s3-write branch from 5b14e7e to 1417696 Compare

September 13, 2023 15:47

akashsha1 reviewed

View reviewed changes

velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp Outdated

+                  VELOX_CHECK(!closed_, "File is closed");
+                  // 'flush' API should trigger uploadPart.
+                  // But upload part if the maximum part size is reached.
+                  if (currentPartSize_ + data.size() > kMaxPartSize) {

Contributor

akashsha1 Sep 13, 2023

should this be >=

Collaborator Author

majetideepak Sep 13, 2023

>= should work according to the documentation. Will do that.

akashsha1 approved these changes

View reviewed changes

majetideepak force-pushed the support-s3-write branch 2 times, most recently from 8978da9 to 7d8716e Compare

September 14, 2023 22:02

Collaborator Author

majetideepak commented Sep 14, 2023 •

edited

@tigrux @akashsha1 @paul-amonson I did some tests at scale and had to fix the semantics a bit.
I observed that the query would fail after writing a 5GiB file. The issue was the difference
between 5 GiB (code limit) vs. 5 GB (AWS limit) upper part limit.
This made me realize that the current TableWriter uses only append() and never a flush() call.
I verified this via logging the number of parts in the close() call.
So each part would have been 5GB big and was being buffered in memory before upload which is bad.
I looked at the latest Arrow S3FS implementation and saw that they use a fixed-size part 10MiB to upload.
I changed the semantics to match this. This made more sense as the write memory is now fixed.
The multi-part upload API also does not match the append() + flush() APIs as it has the min and max part limitations.
Making the append() manage the uploads and flush() a no-op matches the multi-part upload semantics well.
I added some comments to reflect this change.

majetideepak force-pushed the support-s3-write branch from 7d8716e to d654cb8 Compare

September 14, 2023 22:20

Collaborator Author

majetideepak commented Sep 14, 2023

With the new semantics, I verified that we can now write files larger than 5GiB.

Collaborator Author

majetideepak commented Sep 15, 2023

I also looked at the gcs::ObjectWriteStream documentation and it does something similar.
https://cloud.google.com/cpp/docs/reference/storage/latest/classgoogle_1_1cloud_1_1storage_1_1ObjectWriteStream

majetideepak requested review from kgpai and pedroerp

September 15, 2023 13:55

Collaborator Author

majetideepak commented Sep 15, 2023

@pedroerp, @kgpai can you please help merge this? Thanks.

majetideepak added 3 commits

September 15, 2023 09:58


          Add S3 Write support

143c492


          fix S3WriteFile semantics

e13a512


          fix comments

29769d5

majetideepak force-pushed the support-s3-write branch from e75551e to 29769d5 Compare

September 15, 2023 13:58

Collaborator Author

majetideepak commented Sep 15, 2023

The linux-build failure is unrelated.

Contributor

facebook-github-bot commented Sep 15, 2023

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot closed this in

dc1c0c7

facebook-github-bot added the Merged label

Contributor

facebook-github-bot commented Sep 15, 2023

@pedroerp merged this pull request in dc1c0c7.

conbench-facebook bot commented Sep 15, 2023

Conbench analyzed the 1 benchmark run on commit dc1c0c7c.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request


          Add S3FileSystem Write support (facebookincubator#6021)

313ea0a

Summary:
S3WriteFile uses the Apache Arrow implementation as a reference.
AWS C++ SDK allows streaming writes via the MultiPart upload API.
Multipart upload allows you to upload a single object as a set of parts.
Each part is a contiguous portion of the object's data.
While AWS and Minio support different sizes for each
part (only requiring a minimum of 5MB), Certain object stores require that every
part be exactly equal (except for the last part). We set this to 10 MiB, so
that in combination with the maximum number of parts of 10,000, this gives a
file limit of 100k MiB (or about 98 GiB).
You can upload these object parts independently and in any order.
After all parts of your object are uploaded, Amazon S3 assembles these parts
and creates the object.
S3WriteFile is not thread-safe.
UploadPart is currently synchronous during append. Flush is no-op as append
handles all the uploads.

Resolves: facebookincubator#4805

Pull Request resolved: facebookincubator#6021

Reviewed By: kgpai

Differential Revision: D49324662

Pulled By: pedroerp

fbshipit-source-id: f26479058f576a63f7d4fe4527b57bd0aa87ab30

codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request


          Add S3FileSystem Write support (facebookincubator#6021)

5658b96

Summary:
S3WriteFile uses the Apache Arrow implementation as a reference.
AWS C++ SDK allows streaming writes via the MultiPart upload API.
Multipart upload allows you to upload a single object as a set of parts.
Each part is a contiguous portion of the object's data.
While AWS and Minio support different sizes for each
part (only requiring a minimum of 5MB), Certain object stores require that every
part be exactly equal (except for the last part). We set this to 10 MiB, so
that in combination with the maximum number of parts of 10,000, this gives a
file limit of 100k MiB (or about 98 GiB).
You can upload these object parts independently and in any order.
After all parts of your object are uploaded, Amazon S3 assembles these parts
and creates the object.
S3WriteFile is not thread-safe.
UploadPart is currently synchronous during append. Flush is no-op as append
handles all the uploads.

Resolves: facebookincubator#4805

Pull Request resolved: facebookincubator#6021

Reviewed By: kgpai

Differential Revision: D49324662

Pulled By: pedroerp

fbshipit-source-id: f26479058f576a63f7d4fe4527b57bd0aa87ab30

codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request


          Add S3FileSystem Write support (facebookincubator#6021)

690d89b

Summary:
S3WriteFile uses the Apache Arrow implementation as a reference.
AWS C++ SDK allows streaming writes via the MultiPart upload API.
Multipart upload allows you to upload a single object as a set of parts.
Each part is a contiguous portion of the object's data.
While AWS and Minio support different sizes for each
part (only requiring a minimum of 5MB), Certain object stores require that every
part be exactly equal (except for the last part). We set this to 10 MiB, so
that in combination with the maximum number of parts of 10,000, this gives a
file limit of 100k MiB (or about 98 GiB).
You can upload these object parts independently and in any order.
After all parts of your object are uploaded, Amazon S3 assembles these parts
and creates the object.
S3WriteFile is not thread-safe.
UploadPart is currently synchronous during append. Flush is no-op as append
handles all the uploads.

Resolves: facebookincubator#4805

Pull Request resolved: facebookincubator#6021

Reviewed By: kgpai

Differential Revision: D49324662

Pulled By: pedroerp

fbshipit-source-id: f26479058f576a63f7d4fe4527b57bd0aa87ab30

ericyuliu pushed a commit to ericyuliu/velox that referenced this pull request


          Add S3FileSystem Write support (facebookincubator#6021)

75da87f

Summary:
S3WriteFile uses the Apache Arrow implementation as a reference.
AWS C++ SDK allows streaming writes via the MultiPart upload API.
Multipart upload allows you to upload a single object as a set of parts.
Each part is a contiguous portion of the object's data.
While AWS and Minio support different sizes for each
part (only requiring a minimum of 5MB), Certain object stores require that every
part be exactly equal (except for the last part). We set this to 10 MiB, so
that in combination with the maximum number of parts of 10,000, this gives a
file limit of 100k MiB (or about 98 GiB).
You can upload these object parts independently and in any order.
After all parts of your object are uploaded, Amazon S3 assembles these parts
and creates the object.
S3WriteFile is not thread-safe.
UploadPart is currently synchronous during append. Flush is no-op as append
handles all the uploads.

Resolves: facebookincubator#4805

Pull Request resolved: facebookincubator#6021

Reviewed By: kgpai

Differential Revision: D49324662

Pulled By: pedroerp

fbshipit-source-id: f26479058f576a63f7d4fe4527b57bd0aa87ab30

majetideepak deleted the support-s3-write branch

November 8, 2023 11:30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

akashsha1 akashsha1 approved these changes

tigrux tigrux approved these changes

paul-amonson paul-amonson approved these changes

kgpai Awaiting requested review from kgpai

pedroerp Awaiting requested review from pedroerp