Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] introduce new variable fs_watermark for proxy_cache_path

Maxim Dounin
June 05, 2020 09:24AM
Hello!

On Thu, May 07, 2020 at 01:40:54AM +0200, Adam Bambuch wrote:

> # HG changeset patch
> # User Adam Bambuch <adam.bambuch2@gmail.com>
> # Date 1588808163 -7200
> # Thu May 07 01:36:03 2020 +0200
> # Node ID 8d054b64f07457cad2b74376d5f88162c887ba35
> # Parent 716eddd74bc2831537f5b3f7ecd16ad3e516d043
> introduce new variable fs_watermark for proxy_cache_path
>
> This configuration parameter should help with better disk usage,
> especially in environments, where nginx doesn't have a separate
> partition for caching and disk space is shared between multiple apps.
> It could also help in environments, where cache contains many small
> files, and cache loader can't load all the files in a reasonable time
> and therefore max_size is exceeded for many hours after nginx restart.

[...]

Thanks for the patch.

This is something I was thinking about for a while, since such
approach indeed can be better than existing cache clearing in many
cases, even if the cache partition is dedicated to nginx: in
particular, it can be used to keep disk usage under control
regardless of the amount of temporary files kept on the same
partition.

The patch below simplifies the approach, and also fixes some
issues with the patch. Notable changes include:

- The parameter is renamed to "min_free", which is in line with
"max_size" and seems to be much more readable.

- Error reporting is removed and statfs() / statvfs() /
GetDiskFreeSpaceEx() errors, if any, instead result in maximum
amount of free space being reported (and min_free cache clearing
effectively disabled), similarly to how it happens with
ngx_fs_bsize().

- Fixed overflow on 32-bit platforms and incorrect cast in "return
(ssize_t) (fs.f_bavail * fs.f_bsize);" in statfs() and statvfs()
cases.

- Fixed incorrect usage of fs.f_bsize instead of fs.f_frsize in
statvfs() case.

- Fixed configuration parsing to report "not available on this
platform" if statfs() / statvfs() is not present for some
reason.

- Slightly improved ngx_http_file_cache_manager() logic to avoid
reporting very large free space values in debug logs if min_free
is not configured.

- Unused output arguments of GetDiskFreeSpaceEx() replaced with
NULL pointers, since GetDiskFreeSpaceEx() allows this.

- Various style issues fixed.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1591326790 -10800
# Fri Jun 05 06:13:10 2020 +0300
# Node ID 850c0efeaddaf3274f791c90d321d295702bd55c
# Parent 14665e4b377c9eae1f07de003b6248923ca087ee
Cache: introduced min_free cache clearing.

Clearing cache based on free space left on a filesystem is
expected to allow better disk utilization in some cases, notably
when disk space might be also used for something other than nginx
cache (including nginx own temporary files) and while loading
cache (when cache size might be inaccurate for a while, effectively
disabling max_size cache clearing).

Based on a patch by Adam Bambuch.

diff --git a/src/http/ngx_http_cache.h b/src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h
+++ b/src/http/ngx_http_cache.h
@@ -160,6 +160,7 @@ struct ngx_http_file_cache_s {

ngx_path_t *path;

+ off_t min_free;
off_t max_size;
size_t bsize;

diff --git a/src/http/ngx_http_file_cache.c b/src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c
+++ b/src/http/ngx_http_file_cache.c
@@ -1959,7 +1959,7 @@ ngx_http_file_cache_manager(void *data)
{
ngx_http_file_cache_t *cache = data;

- off_t size;
+ off_t size, free;
time_t wait;
ngx_msec_t elapsed, next;
ngx_uint_t count, watermark;
@@ -1988,7 +1988,19 @@ ngx_http_file_cache_manager(void *data)
size, count, (ngx_int_t) watermark);

if (size < cache->max_size && count < watermark) {
- break;
+
+ if (!cache->min_free) {
+ break;
+ }
+
+ free = ngx_fs_available(cache->path->name.data);
+
+ ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
+ "http file cache free: %O", free);
+
+ if (free > cache->min_free) {
+ break;
+ }
}

wait = ngx_http_file_cache_forced_expire(cache);
@@ -2304,7 +2316,7 @@ ngx_http_file_cache_set_slot(ngx_conf_t
{
char *confp = conf;

- off_t max_size;
+ off_t max_size, min_free;
u_char *last, *p;
time_t inactive;
ssize_t size;
@@ -2341,6 +2353,7 @@ ngx_http_file_cache_set_slot(ngx_conf_t
name.len = 0;
size = 0;
max_size = NGX_MAX_OFF_T_VALUE;
+ min_free = 0;

value = cf->args->elts;

@@ -2476,6 +2489,29 @@ ngx_http_file_cache_set_slot(ngx_conf_t
continue;
}

+ if (ngx_strncmp(value[i].data, "min_free=", 9) == 0) {
+
+#if (NGX_WIN32 || NGX_HAVE_STATFS || NGX_HAVE_STATVFS)
+
+ s.len = value[i].len - 9;
+ s.data = value[i].data + 9;
+
+ min_free = ngx_parse_offset(&s);
+ if (min_free < 0) {
+ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+ "invalid min_free value \"%V\"", &value[i]);
+ return NGX_CONF_ERROR;
+ }
+
+#else
+ ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+ "min_free is not supported "
+ "on this platform, ignored");
+#endif
+
+ continue;
+ }
+
if (ngx_strncmp(value[i].data, "loader_files=", 13) == 0) {

loader_files = ngx_atoi(value[i].data + 13, value[i].len - 13);
@@ -2607,6 +2643,7 @@ ngx_http_file_cache_set_slot(ngx_conf_t

cache->inactive = inactive;
cache->max_size = max_size;
+ cache->min_free = min_free;

caches = (ngx_array_t *) (confp + cmd->offset);

diff --git a/src/os/unix/ngx_files.c b/src/os/unix/ngx_files.c
--- a/src/os/unix/ngx_files.c
+++ b/src/os/unix/ngx_files.c
@@ -884,6 +884,19 @@ ngx_fs_bsize(u_char *name)
return (size_t) fs.f_bsize;
}

+
+off_t
+ngx_fs_available(u_char *name)
+{
+ struct statfs fs;
+
+ if (statfs((char *) name, &fs) == -1) {
+ return NGX_MAX_OFF_T_VALUE;
+ }
+
+ return (off_t) fs.f_bavail * fs.f_bsize;
+}
+
#elif (NGX_HAVE_STATVFS)

size_t
@@ -908,6 +921,19 @@ ngx_fs_bsize(u_char *name)
return (size_t) fs.f_frsize;
}

+
+off_t
+ngx_fs_available(u_char *name)
+{
+ struct statvfs fs;
+
+ if (statvfs((char *) name, &fs) == -1) {
+ return NGX_MAX_OFF_T_VALUE;
+ }
+
+ return (off_t) fs.f_bavail * fs.f_frsize;
+}
+
#else

size_t
@@ -916,4 +942,11 @@ ngx_fs_bsize(u_char *name)
return 512;
}

+
+off_t
+ngx_fs_available(u_char *name)
+{
+ return NGX_MAX_OFF_T_VALUE;
+}
+
#endif
diff --git a/src/os/unix/ngx_files.h b/src/os/unix/ngx_files.h
--- a/src/os/unix/ngx_files.h
+++ b/src/os/unix/ngx_files.h
@@ -349,6 +349,7 @@ ngx_int_t ngx_directio_off(ngx_fd_t fd);
#endif

size_t ngx_fs_bsize(u_char *name);
+off_t ngx_fs_available(u_char *name);


#if (NGX_HAVE_OPENAT)
diff --git a/src/os/win32/ngx_files.c b/src/os/win32/ngx_files.c
--- a/src/os/win32/ngx_files.c
+++ b/src/os/win32/ngx_files.c
@@ -658,6 +658,19 @@ ngx_fs_bsize(u_char *name)
}


+off_t
+ngx_fs_available(u_char *name)
+{
+ ULARGE_INTEGER navail;
+
+ if (GetDiskFreeSpaceEx((const char *) name, &navail, NULL, NULL) == 0) {
+ return NGX_MAX_OFF_T_VALUE;
+ }
+
+ return (off_t) navail.QuadPart;
+}
+
+
static ngx_int_t
ngx_win32_check_filename(u_char *name, u_short *u, size_t len)
{
diff --git a/src/os/win32/ngx_files.h b/src/os/win32/ngx_files.h
--- a/src/os/win32/ngx_files.h
+++ b/src/os/win32/ngx_files.h
@@ -259,6 +259,7 @@ ngx_int_t ngx_directio_off(ngx_fd_t fd);
#define ngx_directio_off_n "ngx_directio_off_n"

size_t ngx_fs_bsize(u_char *name);
+off_t ngx_fs_available(u_char *name);


#define ngx_stdout GetStdHandle(STD_OUTPUT_HANDLE)


--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] introduce new variable fs_watermark for proxy_cache_path

Adam Bambuch 449 May 06, 2020 07:42PM

Re: [PATCH] introduce new variable fs_watermark for proxy_cache_path

Maxim Dounin 130 June 05, 2020 09:24AM

Re: [PATCH] introduce new variable fs_watermark for proxy_cache_path

Anonymous User 181 June 05, 2020 11:14AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 214
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready