Use multipartupload for uploading chunks to s3 #2541

mpivchev · 2023-07-19T14:29:32Z

Docs: nextcloud/documentation#10740
Dependency PR: nextcloud/NextcloudKit#38

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

mpivchev · 2023-07-19T14:52:53Z

@marinofaggiana @tobiasKaminsky Why do we need to specify the chunk size manually inside the apps? On web chunking happens automatically.

Also, you have to adjust it constantly if you upload different files. For example I wanted to upload a 20 MB file so I set the chunks to 5MBs, but then I wanted to upload a 20 GB file and I wanted to change them to 1GB. Having this done automatically would be a much better experience. We can still keep the option for manual chunk size as well.

@marinofaggiana Second thing, the docs specify that the min chunk is 5MB and the max can be 5GB. Currently, the max in the app is 100MB. Can we increase it?

mpivchev · 2023-07-19T14:54:53Z

What else to add:

Min chunk size is 5 MiB = 5242880 bytes. Our chunk code works with MBs, so to not overcomplicate things, the min chunk size will be set to 10MB instead.
Change the chunk size UI element to have 10MB as min size.
If a user has currently set their chunk size to <10, we need to reset it.

marinofaggiana · 2023-07-19T15:00:40Z

@mpivchev

yes, we can increase it
if we decide to set it automatically, we have to implement the control in auto upload:
- if the file (image/video) is a chuck file then it is not sent in the background, I can easily handle this one
the upload GUI needs to be changed (Instagram example):

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

juliusknorr · 2023-07-25T12:01:54Z

iOSClient/Utility/CCUtility.m

 + (void)setChunkSize:(NSInteger)size
 {
+    if (size < 10) size = 0;


I think the size should still be default to the minimum supported value of 5MB then if configured to be > 0.

Maybe there is some byte calculation in another place where the 5 MB -> bytes generates the wrong value?

We can remove this two function and use the new algorithm

juliusknorr

Small comment left regarding the chunk size, but tested and works and also verified that the S3 upload mechanism is properly used.

…use-multipartupload-for-uploading-chunks-to-s3-2 Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com> # Conflicts: # Nextcloud.xcodeproj/project.pbxproj

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

mpivchev · 2023-07-25T14:52:09Z

iOSClient/Utility/CCUtility.m

-
-    if (size == nil) {
+    NSInteger sizeInt = [size integerValue];
+
+    if (size == nil || sizeInt < 10) {
        return 0;
    } else {
        return [size integerValue];
    }
 }

+/// In megabytes (MB)
 + (void)setChunkSize:(NSInteger)size
 {
+    if (size < 10) size = 0;


Int checks are temporary until we add auto chunking

mpivchev · 2023-07-25T15:01:34Z

This closes #2471

New things

codecov · 2023-07-25T15:29:33Z

Codecov Report

Patch coverage has no change and project coverage change: +0.55% 🎉

Comparison is base (8dbf5fc) 9.37% compared to head (a9eb4da) 9.92%.

❗ Current head a9eb4da differs from pull request most recent head 3f13bd9. Consider uploading reports for the commit 3f13bd9 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #2541      +/-   ##
==========================================
+ Coverage     9.37%   9.92%   +0.55%     
==========================================
  Files          185     185              
  Lines        26355   26361       +6     
  Branches      9827    9829       +2     
==========================================
+ Hits          2471    2617     +146     
+ Misses       23669   23519     -150     
- Partials       215     225      +10

Files Changed	Coverage Δ
iOSClient/Data/NCManageDatabase.swift	`17.88% <ø> (ø)`
...SClient/Networking/NCNetworkingChunkedUpload.swift	`0.00% <0.00%> (ø)`
iOSClient/Settings/CCAdvanced.m	`0.00% <0.00%> (ø)`
iOSClient/Utility/CCUtility.m	`22.97% <0.00%> (-0.07%)`	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

marinofaggiana · 2023-07-25T15:32:33Z

iOSClient/Utility/CCUtility.m

 + (void)setChunkSize:(NSInteger)size
 {
+    if (size < 10) size = 0;


We can remove this two function and use the new algorithm

mpivchev · 2023-07-25T15:58:46Z

@marinofaggiana Bad idea since we still have the option in settings to change chunks. I added a new issue that will implement this. I suggest we keep this for now. #2547

…use-multipartupload-for-uploading-chunks-to-s3-2 Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com> # Conflicts: # Nextcloud.xcodeproj/project.pbxproj

WIP

b3c51fe

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

mpivchev changed the title ~~WIP~~ S3 Multiupload Jul 19, 2023

mpivchev changed the title ~~S3 Multiupload~~ Use multipartupload for uploading chunks to s3 Jul 19, 2023

mpivchev mentioned this pull request Jul 19, 2023

Use multipartupload for uploading chunks to s3 nextcloud/NextcloudKit#38

Merged

mpivchev mentioned this pull request Jul 19, 2023

New upload GUI #2542

Closed

mpivchev added 2 commits July 19, 2023 17:55

Change stepper values

cbc36b6

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

Add check

191d401

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

mpivchev linked an issue Jul 21, 2023 that may be closed by this pull request

Use MultipartUpload for uploading chunks to s3 #2471

Closed

WIP

a407ba6

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

juliusknorr reviewed Jul 25, 2023

View reviewed changes

juliusknorr previously approved these changes Jul 25, 2023

View reviewed changes

mpivchev added 2 commits July 25, 2023 16:19

Merge branch 'develop' of https://github.com/nextcloud/ios into 2471-…

1d9b218

…use-multipartupload-for-uploading-chunks-to-s3-2 Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com> # Conflicts: # Nextcloud.xcodeproj/project.pbxproj

Finish

873ce8e

Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>

mpivchev commented Jul 25, 2023

View reviewed changes

mpivchev marked this pull request as ready for review July 25, 2023 14:52

mpivchev requested a review from marinofaggiana July 25, 2023 14:53

mpivchev mentioned this pull request Jul 25, 2023

Add automatic chunk sizing, remove manual chunk sizing #2547

Closed

marinofaggiana requested changes Jul 25, 2023

View reviewed changes

Merge branch 'develop' of https://github.com/nextcloud/ios into 2471-…

3f13bd9

…use-multipartupload-for-uploading-chunks-to-s3-2 Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com> # Conflicts: # Nextcloud.xcodeproj/project.pbxproj

marinofaggiana approved these changes Jul 27, 2023

View reviewed changes

mpivchev merged commit adb2909 into develop Jul 27, 2023

delete-merged-branch bot deleted the 2471-use-multipartupload-for-uploading-chunks-to-s3-2 branch July 27, 2023 14:58

joshtrichards mentioned this pull request Sep 21, 2023

Remove warning about disk space needed for uploads to S3 nextcloud/server#40539

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use multipartupload for uploading chunks to s3 #2541

Use multipartupload for uploading chunks to s3 #2541

mpivchev commented Jul 19, 2023 •

edited

Loading

mpivchev commented Jul 19, 2023 •

edited

Loading

mpivchev commented Jul 19, 2023 •

edited

Loading

marinofaggiana commented Jul 19, 2023 •

edited

Loading

juliusknorr Jul 25, 2023

marinofaggiana Jul 25, 2023

juliusknorr left a comment

mpivchev Jul 25, 2023

mpivchev commented Jul 25, 2023

codecov bot commented Jul 25, 2023 •

edited

Loading

marinofaggiana Jul 25, 2023

mpivchev commented Jul 25, 2023

Use multipartupload for uploading chunks to s3 #2541

Use multipartupload for uploading chunks to s3 #2541

Conversation

mpivchev commented Jul 19, 2023 • edited Loading

mpivchev commented Jul 19, 2023 • edited Loading

mpivchev commented Jul 19, 2023 • edited Loading

marinofaggiana commented Jul 19, 2023 • edited Loading

juliusknorr Jul 25, 2023

Choose a reason for hiding this comment

marinofaggiana Jul 25, 2023

Choose a reason for hiding this comment

juliusknorr left a comment

Choose a reason for hiding this comment

mpivchev Jul 25, 2023

Choose a reason for hiding this comment

mpivchev commented Jul 25, 2023

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

marinofaggiana Jul 25, 2023

Choose a reason for hiding this comment

mpivchev commented Jul 25, 2023

mpivchev commented Jul 19, 2023 •

edited

Loading

mpivchev commented Jul 19, 2023 •

edited

Loading

mpivchev commented Jul 19, 2023 •

edited

Loading

marinofaggiana commented Jul 19, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading