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

Fix: Disable proxy_buffering #2178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WolframRavenwolf
Copy link

TL;DR: This PR disables proxy_buffering in Nextcloud's example Docker setups to fix large file (>1GB) download failures over slower network connections, reverting a recent breaking change within nginx-proxy.

Summary

This Pull Request reintroduces a configuration that has been crucial for the stability and functionality of Nextcloud's example Docker setups. It disables proxy_buffering within these configurations, reverting a change made in the December 2023 release (v1.5.0) of nginx-proxy where proxy_buffering was enabled after being disabled for over nine years.

Background

The enablement of proxy_buffering in the recent nginx-proxy release has introduced a significant breaking change affecting file download operations, particularly observed with large files. Files larger than 1 GB are prone to cutoff issues, stemming from the limitations imposed by the default proxy_max_temp_file_size. This has disrupted the expected and long-standing behavior of file downloads through Nextcloud's Docker setups, effectively hindering the reliability of serving large files.

Technical Implications

The re-enablement of proxy_buffering causes nginx to use temporary files for buffering responses from proxied servers. For large downloads, this behavior leads to an accelerated filling of the proxy's temporary storage, resulting in downloads being prematurely terminated once the proxy_max_temp_file_size threshold is reached. Given that increasing the download speeds or the temporary file size is not feasible—due to external network conditions and storage constraints, respectively—the restoration of the pre-v1.5.0 behavior becomes essential.

Proposed Solution

After thorough considerations of the available options and taking into account the successful operation of the previous configuration since 2014, this Pull Request proposes to disable proxy_buffering for Nextcloud's example Docker setups. This change aims to:

  • Ensure the consistency and reliability of file downloads, especially for files exceeding 1 GB.
  • Prevent the unnecessary consumption of temporary storage capacity on the proxy.
  • Align with the proven configuration practices utilized effectively before the December 2023 update.

Conclusion

Reverting the proxy_buffering setting is a targeted measure to maintain the robustness and user satisfaction of Nextcloud's Docker-based deployments, particularly concerning large file handling. I believe this change will restore the expected download functionality and efficiency that we have relied on over the years.

Signed-off-by: Stefan Daniel Schwarz <github.com@wolfram.ravenwolf.de>
Copy link
Member

@joshtrichards joshtrichards left a comment

Choose a reason for hiding this comment

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

I agree that this makes sense to stay in alignment with Nextcloud Server upstream current recommendations: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/big_file_upload_configuration.html#nginx

@joshtrichards
Copy link
Member

Thanks for the detailed context, @StefanDanielSchwarz. Always makes life easier for reviewers! :)

@joshtrichards joshtrichards requested a review from J0WI March 19, 2024 13:05
@J0WI
Copy link
Contributor

J0WI commented Mar 29, 2024

Why is increasing proxy_max_temp_file_size not enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug dependencies examples Compose/Dockerfile/etc integration: proxy Integrating with a reverse proxy regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants