Skip to content
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.

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

Add ENV variables to adjust max_file_uploads and max_chunk_size #5449

Closed
wants to merge 8 commits into from

Conversation

JMarcosHP
Copy link
Contributor

@JMarcosHP JMarcosHP commented Oct 21, 2024

Close #5095

@szaimen szaimen added 3. to review Waiting for reviews enhancement New feature or request labels Oct 21, 2024
@szaimen
Copy link
Collaborator

szaimen commented Oct 21, 2024

Hi @JMarcosHP , thanks a lot for the PR!

Did you already do the following #5095 (reply in thread)? Did it make it work for you? (We need this information in order to judge if it makes sense to merge your PR going forward)

@JMarcosHP
Copy link
Contributor Author

JMarcosHP commented Oct 21, 2024

Hi @JMarcosHP , thanks a lot for the PR!

Did you already do the following #5095 (reply in thread)? Did it make it work for you? (We need this information in order to judge if it makes sense to merge your PR going forward)

Hi, yes I've tested my self that parameters and fixed my issue, hope this will help Cloudflare Tunnel users. By adjusting the chunk size you can improve upload speeds.

…AX_FILE_UPLOADS

Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
…You need a space before the ]. [SC1020]"

Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
@JMarcosHP
Copy link
Contributor Author

I think now this is ready for review and merge.

Signed-off-by: Jehu Marcos Herrera Puentes <[email protected]>
@szaimen
Copy link
Collaborator

szaimen commented Oct 22, 2024

First of all thanks a lot for your PR!

The thing is that with nextcloud/server#45652 (which brings chunking for public uploads) and nextcloud/server#47682 which consists of nextcloud/server#48461 and nextcloud/desktop#7161 that improves the default chunking situation, I'd like to wait for Nextcloud 31 and a Desktop client release that includes the change first and afterwards re-evaluate the situation to check if we actually need this setting afterwards.

I hope this makes sense?

@szaimen szaimen marked this pull request as draft October 22, 2024 09:25
@JMarcosHP
Copy link
Contributor Author

JMarcosHP commented Oct 22, 2024

First of all thanks a lot for your PR!

The thing is that with nextcloud/server#45652 (which brings chunking for public uploads) and nextcloud/server#47682 which consists of nextcloud/server#48461 and nextcloud/desktop#7161 that improves the default chunking situation, I'd like to wait for Nextcloud 31 and a Desktop client release that includes the change first and afterwards re-evaluate the situation to check if we actually need this setting afterwards.

I hope this makes sense?

All right Sir, let's wait for Nextcloud 31. 👌🏻
Just remember, the goal is to let the users:

  • Adjust the default chunk size (by default is 10 MiB on NC server) to increase upload performance.

  • Better handling of /tmp.

@@ -122,6 +123,7 @@ RUN set -ex; \
{ \
echo 'memory_limit=${PHP_MEMORY_LIMIT}'; \
echo 'upload_max_filesize=${PHP_UPLOAD_LIMIT}'; \
echo 'max_file_uploads=${PHP_MAX_FILE_UPLOADS}'; \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so that this info does not get lost: #5095 (reply in thread)

@JMarcosHP JMarcosHP closed this by deleting the head repository Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants