<div dir="ltr"> <br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">my personal preference is to keep disks mirrored</blockquote>Certainly, this is ideal, however Nginx is used by many businesses (cdn, content providers, etc) whose margins are often too low to justify hardware redundancy. And using cheap SSDs can have incredibly bad impact on performance, as we've encountered certain ssds (870 QVO f.e.) that, at the extremes, took (literally) seconds to return from open() / read() / write() <br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">can be reconsidered if there is an understanding that this is quite important in some setups and there is a simple enough way to handle such failures</blockquote><div>We'll need to implement this regardless so I'll, fingers crossed, be back with updates.<br><br></div>Following are notes on my initial idea which proved too problematic to pursue further, so feel free to skip.<br>Create the temp file ahead of time, before the proxy_no_cache check, and take the proxy_no_cache code path if we fail to create the file.<br>Clearly not very elegant either as:<br>The temp file will need to be removed in the else of "if (p->cacheable) ..."<br>The temp file may be created successfully yet you may still encounter an IO error a few nanoseconds later when you try to write to it.<br>There were several variants of this (diff file attached) which either leaked tempfiles or broke proxy_no_cache. <div><div> </div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 31, 2025 at 11:36 AM Maxim Dounin <<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello!<br>
<br>
On Thu, Jan 30, 2025 at 11:01:32AM +0200, Clima Gabriel wrote:<br>
<br>
> Hello Maxim,<br>
> Hope this helps.<br>
> We encounter disk failures fairly often and what the kernel will do most of<br>
> the time is re-mount the disk as read-only.<br>
> What I did was add this check which checks if the disk is healthy before<br>
> hand and executes the proxy_no_cache path if it is.<br>
> It doesn't cover all the possible disk failures, like sometimes you'll just<br>
> get IO errors and still return 5XX to clients. But to cover all cases a<br>
> much cleverer reshuffling of the code is needed.<br>
> <br>
> <br>
> diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c<br>
> index d7f427d50..839ed6c0d 100644<br>
> --- a/src/http/ngx_http_upstream.c<br>
> +++ b/src/http/ngx_http_upstream.c<br>
> @@ -8,6 +8,7 @@<br>
> #include <ngx_config.h><br>
> #include <ngx_core.h><br>
> #include <ngx_http.h><br>
> +#include <sys/statvfs.h><br>
> <br>
> #include <ngx_http_proxy_module.h><br>
> <br>
> @@ -3424,6 +3425,16 @@ ngx_http_upstream_send_response(ngx_http_request_t<br>
> *r, ngx_http_upstream_t *u)<br>
> break;<br>
> <br>
> default: /* NGX_OK */<br>
> + if (r->cache) {<br>
> + struct statvfs fs;<br>
> + if (statvfs((char *)r->cache->file_cache->path->name.data,<br>
> &fs) == -1) {<br>
> + return NGX_ERROR;<br>
> + }<br>
> + if ((fs.f_flag & ST_RDONLY) != 0) {<br>
> + u->cacheable = 0;<br>
> + break;<br>
> + }<br>
> + }<br>
> <br>
> if (u->cache_status == NGX_HTTP_CACHE_BYPASS) {<br>
<br>
I cannot say I like this approach.<br>
<br>
Rather, I would recommend adding an explicit test via the <br>
proxy_no_cache configuration directive if that's an expected state <br>
in your setup. Right now this certainly can be done with the <br>
embedded Perl module, but we can consider extending "if" with "-r" <br>
and "-w" tests to make things easier.<br>
<br>
Alternatively, we can consider handling (at least some) cache <br>
access failures gracefully and ensure that we'll fall back to <br>
normal proxying if this happens. This wasn't done previously to <br>
keep the code simple, but can be reconsidered if there is an <br>
understanding that this is quite important in some setups and <br>
there is a simple enough way to handle such failures.<br>
<br>
Please also note that right now even proxying won't work if <br>
a temporary file is needed and cannot be created, as file access <br>
failures are considered fatal.<br>
<br>
OTOH, my personal preference is to keep disks mirrored, this <br>
ensures that a single disk failure won't affect server operations <br>
and provides better performance as a bonus.<br>
<br>
(Note well that the code in question was modified in freenginx to <br>
address the issue reported by Kirill in the thread you are <br>
replying to, and your patch won't apply, see <br>
<a href="https://freenginx.org/hg/nginx/rev/c5623963c29e" rel="noreferrer" target="_blank">https://freenginx.org/hg/nginx/rev/c5623963c29e</a> for details.)<br>
<br>
[...]<br>
<br>
-- <br>
Maxim Dounin<br>
<a href="http://mdounin.ru/" rel="noreferrer" target="_blank">http://mdounin.ru/</a><br>
_______________________________________________<br>
nginx mailing list<br>
<a href="mailto:nginx@nginx.org" target="_blank">nginx@nginx.org</a><br>
<a href="https://mailman.nginx.org/mailman/listinfo/nginx" rel="noreferrer" target="_blank">https://mailman.nginx.org/mailman/listinfo/nginx</a><br>
</blockquote></div>