-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
improve some general documentation #613
Conversation
How did you determine your proxy_buffering_size settings if you changed it at all? |
I read the article that you added and it encourages adjusting the buffer size to the likely maximum HTTP header size a request can reach to prevent Nginx from temporarily writing a request to disk. It also recommends disabling proxy_buffering by default if Nginx is not used for caching and just as a TLS termination point. It would nice to determine a typical HTTP header size and make recommendations for good defaults based on those. |
//> How did you determine your proxy_buffering_size settings if you changed it at all? Good question. Since we’re talking about uploading large files, however, the relevant directive is But he also said
One can also turn buffering off if one doesn’t need to deal with slow clients. |
//I’ve removed reference to Vershinin’s article because it’s focussed on I’ve found time to do extensive testing for uploading to conduwuit under different scenarios. My conclusions are as follows: The only directive that’s absolutely required is
I’ve updated my patch accordingly. |
Just saw the new version
What you’re seeing here is disk buffering. It’s caused by the typical upload exceeding the default 8x4k in-memory nginx throws two kinds of warnings when this happens
and when downloaded from conduwuit
|
Corrections/changes/clarifications are welcome |
Basically I guess the one thing we really need to put in there is After that, the user has three choices:
3 may suit conduwuit instances with a couple of users, whilst high concurrency servers should probably go for a compromise between 1 and 2. I gather we can just spell out the trade-offs and let admins decide for themselves. Could you provide some feedback as to your preference so I know what to do with this PR? |
I missed that part. Since the Nginx is public facing to the Internet, it is possible to encounter slow clients. Hmm yeah keeping buffering on and increasing buffer size seems like a better idea. |
I think Nginx could use its own section just like Caddy. I also think it would be cool if we could recommend defaults. I can post the settings that I use but they are educated but also arbitrary:
I keep proxy_buffer_size at its default of the arch page_size. Same with proxy_busy_buffer size which on x86_64 would imply 4k and 68k for each. My values assume that one may upload files to the Conduwuit server, hence the bigger buffer size. Also very much a nitpick (and i am not a maintainer here): it would be nicer to split up the |
I would prefer not. Having a bunch of sections on reverse proxies was intentionally removed and leaving only a Caddy section was added to steer users towards using a simple, fast, no-headache, and effective reverse proxy unlike Nginx/Apache. The most I am willing to do, and have been doing, is having "reverse proxy caveats" like:
If you feel like "reverse proxy caveats" should be in its own section, I'm up for doing that. But I am not going to steer users towards Nginx and they should be reviewing the 100000+ "nginx reverse proxy" examples out there if they want to use Nginx instead of Caddy. |
That feels appropriate. Or something like "Caddy" |
A common pattern you want to see in docs is this. It repeats over each section or chapter.
For the reverse proxy section that would mean:
|
This is what I'm fine with doing. |
Since what June wants is absolutely minimal caveat about non-Caddy proxies, I’ve left only the part about matching The whole buffering thing really depends on the user’s environment. The default @girlbossceo, does this look good to you? |
Where might I find more details about that? lighttpd does not modify the |
@nisbet-hubbard I'll review it later, but please rebase this PR onto |
d5d7afc
to
0907205
Compare
0907205
to
26bcc7e
Compare
Done. |
I'm kinda confused as to why the suggestion of disabling The advice was added because multiple Nginx users have reported sporadic media failures, both downloading remote media and uploading media, which were all solved by disabling I'm not sure if my comment about going against creating a dedicated Nginx section was misinterpreted, but it wasn't saying "I don't want this tiny piece of advice to tune buffers". It was to avoid creating extensive amounts of documentation on one single reverse proxy software that's needlessly complex for the majority of users, while also duplicating the 1000+ "nginx reverse proxy example" guides out there, so much that it needs its own page/section. So, either this PR needs to put back the advice of disabling |
Thanks for filling us in on the background! I agree with your conclusion about the two paths forward. From what you’re saying, I suppose what we want to avoid is adding these band-aid solutions to the docs whenever users discover a workaround to their local nginx configuration problems, usually without even identifying the real problem. The advice about What’s your thoughts on this? |
The way I decide for me to add those tiny bits of ad-hoc reverse proxy advice like the Apache, Lighttpd, and Nginx ones are if they're surprisingly a common issue. Like the proxy_buffering one, there were a good portion of unique users joining the Matrix room and complaining about media download issues that conduwuit logs couldn't do anything but return a 5xx error code (from the reverse proxy itself). It was only until after these users dug into their Nginx logs and found that disabling proxy_buffering solved the issue. Since it seemed to be a pretty frequent issue, that was why I added it. I insisted these users just use Caddy, but they were set on using Nginx. Same with the Or if they're critical configuration changes that need to be made to function properly, like Apache having to use So, I do agree that these users should just be using Caddy, or spend some time trying to figure out the issue themselves through Nginx logs, and asking for Nginx support elsewhere, but if it's a really common issue then that would be where I'd like that advice to be under the "reverse proxy caveats" section if there was one that would be made. I guess I'm fine with removing the disable |
Yeah, makes sense to me. Can definitely sympathise with your having to accommodate users who neither wish to spend time debugging their nginx nor switch to Caddy. Also read about bikeshedding re broken X-Matrix signatures over at Conduit.
Maybe you can mention this in the docs so first-time users know they don’t need the
It may help if the next time this comes up you ask the reporter to do |
I assume the suggestion of
proxy_buffering off;
was prompted by a warning such asan upstream response is buffered to a temporary file
because the image file exceeds the (8 4k|8k) in-memory buffer size. In which case the user should probably increase the default buffer size in the first instance (if there’s enough RAM) rather than turning off buffering to suppress the warning. I’m linking to Vershinin‘s article because it seems to be the most thorough one on this subject.adduser
is a symlink touseradd
on recent Fedora/RHEL and doesn’t accept the same flags as Debian so I propose removing them from that paragraph.