On a production environment with a given maxconn, the fd limit can be
successfully set at a given time. While raising maxconn, a new max fd
limit is calculated. If the setrlimit fails (e.g if sysctl fs.nr_open
is lower), we silently revert to the max obtained through getrlimit,
which can be way lower than the previous one set, with a working maxconn;
we only deliver a warning at startup.
In our production environment we rollout new configuration through
progressive reload. If the reload/restart fails (i.e if the health check
fails), the rollout is stopped. Before this patch, the new maxconn is
applied silently whether setrlimit succeed or not; i.e you might easily
face a case where fd limit is reverted to rlim_max making your
production environment unstable very quickly.

That's why we propose to explicitly fail when fd limit cannot be raised
to avoid bad surprises when a new maxconn is set.
We extended the failing case to all setrlimit. This seem to be more in
correlation with other config errors where haproxy immediately fails.

This can been seen as a partial revert of commit
164dd0b6e4ad (BUG/MINOR: init: ensure that FD limit is raised to the max
allowed)

We also understand this is a change of behaviour and you can see that as
a starting point of a conversation to better handle those breaking
cases. But the general idea is to be coherent with other failing config
errors.

Signed-off-by: William Dauchy <w.dau...@criteo.com>
---
 src/haproxy.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index a7f294d6a..e8b855053 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2829,14 +2829,11 @@ int main(int argc, char **argv)
                limit.rlim_max = MAX(rlim_fd_max_at_boot, limit.rlim_cur);
 
                if (setrlimit(RLIMIT_NOFILE, &limit) == -1) {
-                       /* try to set it to the max possible at least */
                        getrlimit(RLIMIT_NOFILE, &limit);
-                       limit.rlim_cur = limit.rlim_max;
-                       if (setrlimit(RLIMIT_NOFILE, &limit) != -1)
-                               getrlimit(RLIMIT_NOFILE, &limit);
-
-                       ha_warning("[%s.main()] Cannot raise FD limit to %d, 
limit is %d.\n", argv[0], global.rlimit_nofile, (int)limit.rlim_cur);
-                       global.rlimit_nofile = limit.rlim_cur;
+                       ha_alert("[%s.main()] Cannot raise FD limit to %d, 
limit is %d.\n",
+                                argv[0], global.rlimit_nofile, 
(int)limit.rlim_cur);
+                       if (!(global.mode & MODE_MWORKER))
+                               exit(1);
                }
        }
 
@@ -2845,13 +2842,17 @@ int main(int argc, char **argv)
                        global.rlimit_memmax * 1048576ULL;
 #ifdef RLIMIT_AS
                if (setrlimit(RLIMIT_AS, &limit) == -1) {
-                       ha_warning("[%s.main()] Cannot fix MEM limit to %d 
megs.\n",
-                                  argv[0], global.rlimit_memmax);
+                       ha_alert("[%s.main()] Cannot fix MEM limit to %d 
megs.\n",
+                                argv[0], global.rlimit_memmax);
+                       if (!(global.mode & MODE_MWORKER))
+                               exit(1);
                }
 #else
                if (setrlimit(RLIMIT_DATA, &limit) == -1) {
-                       ha_warning("[%s.main()] Cannot fix MEM limit to %d 
megs.\n",
-                                  argv[0], global.rlimit_memmax);
+                       ha_alert("[%s.main()] Cannot fix MEM limit to %d 
megs.\n",
+                                argv[0], global.rlimit_memmax);
+                       if (!(global.mode & MODE_MWORKER))
+                               exit(1);
                }
 #endif
        }
-- 
2.23.0


Reply via email to