You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "klsince (via GitHub)" <gi...@apache.org> on 2023/02/10 06:40:55 UTC
[GitHub] [pinot] klsince opened a new pull request, #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
klsince opened a new pull request, #10263:
URL: https://github.com/apache/pinot/pull/10263
Add support to upload S3 object in multi parts but sequentially in this PR, mainly to overcome the 5GB limit of single PutObject call. This can be extended to upload parts in parallel if needed.
## Release Note ##
One major config to init S3PinotFS
1. minObjectSizeForMultiPartUpload: -1 by default to disable multi part upload. One can set it to something like 2G to upload large object by multi parts;
2. multiPartUploadPartSize: 128MB by default, no less than 5MB according to S3 doc.
3. multiPartUploadMaxPartNumAllowed: must be 10000 (default) or less according to S3 doc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on a diff in pull request #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #10263:
URL: https://github.com/apache/pinot/pull/10263#discussion_r1103352879
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3Config.java:
##########
@@ -49,6 +50,13 @@ public class S3Config {
public static final String EXTERNAL_ID = "externalId";
public static final String SESSION_DURATION_SECONDS = "sessionDurationSeconds";
public static final String ASYNC_SESSION_UPDATED_ENABLED = "asyncSessionUpdateEnabled";
+ public static final String MIN_OBJECT_SIZE_FOR_MULTI_PART_UPLOAD = "minObjectSizeForMultiPartUpload";
+ public static final String MULTI_PART_UPLOAD_PART_SIZE = "multiPartUploadPartSize";
+ public static final String MULTI_PART_UPLOAD_MAX_PART_NUM_ALLOWED = "multiPartUploadMaxPartNumAllowed";
Review Comment:
I was thinking this config may be helpful when S3 side updates this limit. But a constant works anyway, as it'd be rare for that to change. I'll add a constant for the 5MB part size limit too for the config validation said below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10263:
URL: https://github.com/apache/pinot/pull/10263#issuecomment-1425295805
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10263](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a555c6) into [master](https://codecov.io/gh/apache/pinot/commit/f17c82004123315a5c0a5539e942934f38c96df9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f17c820) will **decrease** coverage by `56.64%`.
> The diff coverage is `64.06%`.
```diff
@@ Coverage Diff @@
## master #10263 +/- ##
=============================================
- Coverage 70.35% 13.72% -56.64%
+ Complexity 5756 182 -5574
=============================================
Files 2016 1962 -54
Lines 109094 106835 -2259
Branches 16583 16319 -264
=============================================
- Hits 76757 14661 -62096
- Misses 26960 91018 +64058
+ Partials 5377 1156 -4221
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `13.72% <64.06%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/plugin/filesystem/S3Config.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1maWxlLXN5c3RlbS9waW5vdC1zMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2ZpbGVzeXN0ZW0vUzNDb25maWcuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../org/apache/pinot/plugin/filesystem/S3PinotFS.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1maWxlLXN5c3RlbS9waW5vdC1zMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2ZpbGVzeXN0ZW0vUzNQaW5vdEZTLmphdmE=) | `45.19% <74.54%> (+4.54%)` | :arrow_up: |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/common/CustomObject.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vQ3VzdG9tT2JqZWN0LmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1592 more](https://codecov.io/gh/apache/pinot/pull/10263?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10263:
URL: https://github.com/apache/pinot/pull/10263#discussion_r1103324593
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3Config.java:
##########
@@ -91,7 +102,13 @@ public S3Config(PinotConfiguration pinotConfig) {
Integer.parseInt(pinotConfig.getProperty(SESSION_DURATION_SECONDS, DEFAULT_SESSION_DURATION_SECONDS));
_asyncSessionUpdateEnabled = Boolean.parseBoolean(
pinotConfig.getProperty(ASYNC_SESSION_UPDATED_ENABLED, DEFAULT_ASYNC_SESSION_UPDATED_ENABLED));
-
+ // non-positive values to disable multipart upload.
+ _minObjectSizeForMultiPartUpload =
+ DataSizeUtils.toBytes(pinotConfig.getProperty(MIN_OBJECT_SIZE_FOR_MULTI_PART_UPLOAD, "-1"));
+ _multiPartUploadPartSize = DataSizeUtils.toBytes(
Review Comment:
Should we validate this size to be larger than 5MB?
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3Config.java:
##########
@@ -49,6 +50,13 @@ public class S3Config {
public static final String EXTERNAL_ID = "externalId";
public static final String SESSION_DURATION_SECONDS = "sessionDurationSeconds";
public static final String ASYNC_SESSION_UPDATED_ENABLED = "asyncSessionUpdateEnabled";
+ public static final String MIN_OBJECT_SIZE_FOR_MULTI_PART_UPLOAD = "minObjectSizeForMultiPartUpload";
+ public static final String MULTI_PART_UPLOAD_PART_SIZE = "multiPartUploadPartSize";
+ public static final String MULTI_PART_UPLOAD_MAX_PART_NUM_ALLOWED = "multiPartUploadMaxPartNumAllowed";
Review Comment:
This seems like a limitation associated with S3, and I don't see the value of making it configurable
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3PinotFS.java:
##########
@@ -551,11 +569,74 @@ public void copyToLocalFile(URI srcUri, File dstFile)
@Override
public void copyFromLocalFile(File srcFile, URI dstUri)
throws Exception {
- LOGGER.info("Copy {} from local to {}", srcFile.getAbsolutePath(), dstUri);
- URI base = getBase(dstUri);
- String prefix = sanitizePath(base.relativize(dstUri).getPath());
- PutObjectRequest putObjectRequest = generatePutObjectRequest(dstUri, prefix);
- _s3Client.putObject(putObjectRequest, srcFile.toPath());
+ if (_minObjectSizeToUploadInParts > 0 && srcFile.length() > _minObjectSizeToUploadInParts) {
+ LOGGER.info("Copy {} from local to {} in parts", srcFile.getAbsolutePath(), dstUri);
+ uploadFileInParts(srcFile, dstUri);
+ } else {
+ LOGGER.info("Copy {} from local to {}", srcFile.getAbsolutePath(), dstUri);
+ String prefix = sanitizePath(getBase(dstUri).relativize(dstUri).getPath());
+ PutObjectRequest putObjectRequest = generatePutObjectRequest(dstUri, prefix);
+ _s3Client.putObject(putObjectRequest, srcFile.toPath());
+ }
+ }
+
+ private void uploadFileInParts(File srcFile, URI dstUri)
+ throws Exception {
+ String bucket = dstUri.getHost();
+ String prefix = sanitizePath(getBase(dstUri).relativize(dstUri).getPath());
+ CreateMultipartUploadResponse multipartUpload =
+ _s3Client.createMultipartUpload(CreateMultipartUploadRequest.builder().bucket(bucket).key(prefix).build());
+ String uploadId = multipartUpload.uploadId();
+ // Upload parts sequentially to overcome the 5GB limit of a single PutObject call.
+ // TODO: parts can be uploaded in parallel for higher throughput, given a thread pool.
+ try (FileInputStream inputStream = FileUtils.openInputStream(srcFile)) {
+ long totalUploaded = 0;
+ long fileSize = srcFile.length();
+ // The part number must start from 1 and no more than the max part num allowed, 10000 by default.
+ // The default configs can upload a single file of 1TB, so the if-branch should rarely happen.
+ int partNum = 1;
+ long partSizeToUse = _multiPartUploadPartSize;
+ if (partSizeToUse * _multiPartUploadMaxPartNumAllowed < fileSize) {
+ partSizeToUse = (fileSize + _multiPartUploadPartSize) / _multiPartUploadMaxPartNumAllowed;
Review Comment:
This seems not correct. Should this be `(fileSize + 9999) / 10000`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on a diff in pull request #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #10263:
URL: https://github.com/apache/pinot/pull/10263#discussion_r1103375647
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3PinotFS.java:
##########
@@ -551,11 +569,74 @@ public void copyToLocalFile(URI srcUri, File dstFile)
@Override
public void copyFromLocalFile(File srcFile, URI dstUri)
throws Exception {
- LOGGER.info("Copy {} from local to {}", srcFile.getAbsolutePath(), dstUri);
- URI base = getBase(dstUri);
- String prefix = sanitizePath(base.relativize(dstUri).getPath());
- PutObjectRequest putObjectRequest = generatePutObjectRequest(dstUri, prefix);
- _s3Client.putObject(putObjectRequest, srcFile.toPath());
+ if (_minObjectSizeToUploadInParts > 0 && srcFile.length() > _minObjectSizeToUploadInParts) {
+ LOGGER.info("Copy {} from local to {} in parts", srcFile.getAbsolutePath(), dstUri);
+ uploadFileInParts(srcFile, dstUri);
+ } else {
+ LOGGER.info("Copy {} from local to {}", srcFile.getAbsolutePath(), dstUri);
+ String prefix = sanitizePath(getBase(dstUri).relativize(dstUri).getPath());
+ PutObjectRequest putObjectRequest = generatePutObjectRequest(dstUri, prefix);
+ _s3Client.putObject(putObjectRequest, srcFile.toPath());
+ }
+ }
+
+ private void uploadFileInParts(File srcFile, URI dstUri)
+ throws Exception {
+ String bucket = dstUri.getHost();
+ String prefix = sanitizePath(getBase(dstUri).relativize(dstUri).getPath());
+ CreateMultipartUploadResponse multipartUpload =
+ _s3Client.createMultipartUpload(CreateMultipartUploadRequest.builder().bucket(bucket).key(prefix).build());
+ String uploadId = multipartUpload.uploadId();
+ // Upload parts sequentially to overcome the 5GB limit of a single PutObject call.
+ // TODO: parts can be uploaded in parallel for higher throughput, given a thread pool.
+ try (FileInputStream inputStream = FileUtils.openInputStream(srcFile)) {
+ long totalUploaded = 0;
+ long fileSize = srcFile.length();
+ // The part number must start from 1 and no more than the max part num allowed, 10000 by default.
+ // The default configs can upload a single file of 1TB, so the if-branch should rarely happen.
+ int partNum = 1;
+ long partSizeToUse = _multiPartUploadPartSize;
+ if (partSizeToUse * _multiPartUploadMaxPartNumAllowed < fileSize) {
+ partSizeToUse = (fileSize + _multiPartUploadPartSize) / _multiPartUploadMaxPartNumAllowed;
Review Comment:
duh... 😂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #10263: upload large s3 object by multi parts to overcome the 5GB limit of PutObject
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10263:
URL: https://github.com/apache/pinot/pull/10263
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org