From 14d7531004c9299c581769843cbd73c49f55bdfd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 20:28:23 +0000 Subject: [PATCH] feat(broker): add [broker] max_open_files and explain descriptor exhaustion Fleet size is bounded by the broker's open-file limit, not by anything in libzmq. Every connected agent holds two of the broker's descriptors for as long as it stays connected -- its publisher on the XSUB frontend, its subscriber on the XPUB backend -- plus a third while it fetches its settings. With the usual soft RLIMIT_NOFILE of 1024 and ~30 descriptors of broker overhead, that caps a fleet at roughly 495 agents. Past that ceiling the broker failed in the least helpful way possible. libzmq's tcp_listener_t::accept() counts EMFILE/ENFILE among the errnos it tolerates (src/tcp_listener.cpp:193-199 in the pinned v4.3.5), so it raised ZMQ_EVENT_ACCEPT_FAILED -- which nothing listened for -- and refused every new agent in silence, while the still-readable listener re-fired the failing accept as fast as it could poll. The only thing that printed was the service-discovery thread, whose once-a-second getifaddrs() and broadcast sockets are the broker's sole timed descriptor allocations and so failed first. The result was a broker that looked healthy while turning agents away, reporting "getifaddrs failed: Too many open files" -- pointing at discovery rather than at the limit actually responsible. Three changes, all backwards-compatible: - New src/detail/fd_limit.hpp resolves, plans and applies the limit. The decision logic is pure -- it takes the observed limits as data and makes no syscalls -- so every branch, including the platforms a given build is not running on, is unit-testable. Only rlim_cur is ever raised, since raising rlim_max needs CAP_SYS_RESOURCE; the target is capped by kern.maxfilesperproc on macOS and fs.nr_open on Linux, both of which setrlimit rejects values above even when rlim_max is RLIM_INFINITY. Windows reports the concept as not applicable rather than pretending: libzmq uses SOCKET handles there, and _setmaxstdio() governs only stdio streams. - [broker] max_open_files raises the soft limit at startup, before anything binds. Unset (the default) leaves it untouched and only reports it, warning when raising would help; 0 asks for as much as the process is permitted; a value above the hard limit is clamped with a warning, and a negative one is ignored with a warning rather than refused, matching io_threads' posture. - The broker now watches its bound sockets for ZMQ_EVENT_ACCEPT_FAILED and reports a refused agent explicitly -- endpoint, limit in force, the agent count it allows, and how to raise it -- rate-limited, and latched per failure class so a routine ECONNABORTED cannot consume the one-shot descriptor-exhaustion explanation. SocketMonitor gains LinkEvent:: AcceptFailed, LinkState::last_event_value (the errno libzmq reports) and a monotonic accept_failures counter; an accept failure deliberately does not move LinkStatus, since it describes the listener rather than any link. - Any discovery socket error caused by a full descriptor table now says so, and the advertising loop reports an unchanged failure at most once every 30 seconds instead of every second. The shipped systemd template now sets LimitNOFILE=65536; without it a unit inherits systemd's DefaultLimitNOFILE, which is that same 1024 on most distributions. `mads doctor` gains a check reporting the limit in force and whether it leaves room for the fleet. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TpZK4EKXZooaAXr1ASFHwo --- CHANGES.md | 43 +++++ mads.ini | 7 + share/man/mads-broker.md | 26 +++ share/templates/service.tpl | 7 + src/detail/fd_limit.hpp | 316 +++++++++++++++++++++++++++++++++++ src/doctor_checks.cpp | 62 +++++++ src/doctor_checks.hpp | 19 +++ src/main/broker.cpp | 195 +++++++++++++++++++++ src/main/doctor.cpp | 12 ++ src/service_discovery.cpp | 56 ++++++- src/socket_monitor.cpp | 20 +++ src/socket_monitor.hpp | 16 ++ tests/test_doctor_checks.cpp | 51 ++++++ tests/test_fd_limit.cpp | 207 +++++++++++++++++++++++ 14 files changed, 1033 insertions(+), 4 deletions(-) create mode 100644 src/detail/fd_limit.hpp create mode 100644 tests/test_fd_limit.cpp diff --git a/CHANGES.md b/CHANGES.md index 885a490..66247ab 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -185,6 +185,49 @@ exactly as before until you opt in. Details and examples are in load time. The same file therefore scales identically under the Director GUI and under `mads up` / `mads doctor --plan`. +- **`[broker] max_open_files`, and a broker that says when it runs out of + descriptors.** Fleet size is bounded by the broker's open-file limit, not by + anything in libzmq: every connected agent holds two of the broker's + descriptors for as long as it stays connected -- its publisher on the XSUB + frontend, its subscriber on the XPUB backend -- plus a third while it fetches + its settings. With the usual soft `RLIMIT_NOFILE` of 1024 that caps a fleet + at roughly **495 agents**. + + Past that ceiling the broker used to fail in the least helpful way possible. + libzmq's TCP listener counts `EMFILE`/`ENFILE` among the errnos `accept()` + may fail with harmlessly, so it raised `ZMQ_EVENT_ACCEPT_FAILED` -- which + nothing listened for -- and refused every new agent in complete silence. The + only thing that printed was the service-discovery thread, because its + once-a-second `getifaddrs()` and broadcast sockets are the broker's only + *timed* descriptor allocations and so were the first to fail. The result was + a broker that looked healthy while turning agents away, reporting + `ServiceDiscovery advertising failed: getifaddrs failed: Too many open + files`, which points at discovery rather than at the limit actually + responsible. + + Three changes, all backwards-compatible: + - `max_open_files` under `[broker]` raises the soft limit at startup, before + any socket is bound. Unset (the default) leaves it untouched and merely + reports it; `0` asks for as much as the process is permitted. It cannot + exceed the hard limit -- a larger value is clamped with a warning, since + only `LimitNOFILE=` or a privileged `ulimit -Hn` can lift that. Portable: + the raise is capped by `kern.maxfilesperproc` on macOS and `fs.nr_open` on + Linux, and reported as not applicable on Windows, which has no per-process + descriptor limit for sockets. + - The broker now watches its bound sockets for `ZMQ_EVENT_ACCEPT_FAILED` and + reports a refused agent explicitly, naming the endpoint, the limit in + force, the agent count it allows and how to raise it. Rate-limited, + because a full descriptor table leaves the listening socket permanently + readable and libzmq retries the failing accept as fast as it can poll. + - Any discovery socket error caused by a full descriptor table now says so, + and the advertising loop reports an unchanged failure at most once every + 30 seconds instead of every second. + + The shipped systemd template (`mads service`) now sets `LimitNOFILE=65536`; + without it a unit inherits systemd's `DefaultLimitNOFILE`, which is that same + 1024 on most distributions. `mads doctor` gained a check reporting the limit + in force and whether it leaves room for the fleet. + ## Fixes - **Broker `p`/`r` keys now actually pause and resume.** The interactive diff --git a/mads.ini b/mads.ini index c34362f..1c420ca 100644 --- a/mads.ini +++ b/mads.ini @@ -73,6 +73,13 @@ prefer_loopback_for_local_services = true # endpoint. One agent fetching a large plugin attachment no longer blocks # every other agent's settings request behind it; see `man mads-broker`. # settings_workers = 2 +# Open file descriptors the broker may use, which is what bounds fleet size: +# every connected agent costs two of them (its publisher and its subscriber), +# so the usual soft limit of 1024 stops at roughly 495 agents. Unset leaves the +# limit alone and just reports it at startup; 0 asks for as many as the OS +# allows. Cannot exceed the hard limit (LimitNOFILE= in the systemd unit, or +# `ulimit -Hn`), and has no effect on Windows. See `man mads-broker`. +# max_open_files = 65536 [logger] diff --git a/share/man/mads-broker.md b/share/man/mads-broker.md index 017e68b..d3ae813 100644 --- a/share/man/mads-broker.md +++ b/share/man/mads-broker.md @@ -110,6 +110,32 @@ unedited settings file behaves exactly as before they existed. agent's plain REQ settings request works unchanged. A value below 1 is clamped to 1 with a warning rather than refused. +**max_open_files** (`[broker]`, integer, unset by default) +: Open file descriptors the broker process may use, which is what bounds + fleet size. Every connected agent holds **two** of the broker's descriptors + for as long as it stays connected -- its publisher on the XSUB frontend and + its subscriber on the XPUB backend -- plus a third while it fetches its + settings. With the usual soft limit of 1024 and the broker's own ~30 + descriptors of overhead, that caps a fleet at roughly **495 agents**, and + libzmq refuses everything past it *almost silently*: its listener treats + `EMFILE` as a non-fatal `accept()` error. Set this and the broker raises its + own soft limit at startup, before binding anything, and reports the + resulting ceiling and the agent count it implies. + + Unset leaves the limit exactly as inherited and only reports it, warning + when it is low enough to be worth raising. `0` asks for as many descriptors + as the process is permitted. **Cannot exceed the hard limit** -- a larger + value is clamped with a warning, because raising the hard limit needs + `LimitNOFILE=` in the systemd unit or a privileged `ulimit -Hn`, not a + settings key. A negative value is ignored with a warning rather than + refused, like `io_threads`. + + Has no effect on Windows, which has no per-process descriptor limit for + sockets, and is reported as not applicable there. On macOS the raise is + additionally capped by `kern.maxfilesperproc`, and on Linux by + `fs.nr_open`. `mads doctor` reports the limit in force and whether it + leaves room for the fleet. + # BUGS The upstream bug tracker can be found at https://github.com/pbosetti/MADS/issues. diff --git a/share/templates/service.tpl b/share/templates/service.tpl index a6fb6c8..f387962 100644 --- a/share/templates/service.tpl +++ b/share/templates/service.tpl @@ -22,6 +22,13 @@ Type=simple Restart=always RestartSec=1 User=root +# systemd hands a unit that does not say otherwise its DefaultLimitNOFILE soft +# value, which is 1024 on most distributions. The broker holds two descriptors +# per connected agent (its publisher and its subscriber), so that default caps +# a fleet at roughly 495 agents -- and libzmq refuses everything past it almost +# silently. Can also be set from the settings file with [broker] max_open_files, +# but only up to the hard limit this line establishes. +LimitNOFILE=65536 ExecStart={{command}} [Install] diff --git a/src/detail/fd_limit.hpp b/src/detail/fd_limit.hpp new file mode 100644 index 0000000..cb94036 --- /dev/null +++ b/src/detail/fd_limit.hpp @@ -0,0 +1,316 @@ +/* +Internal helper: reads, plans and applies the process-wide open-file-descriptor +limit (POSIX RLIMIT_NOFILE), backing the `[broker] max_open_files` setting. +Not part of the installed SDK (src/detail/ is excluded from the LIB_HEADERS +install glob in CMakeLists.txt). + +WHY THIS EXISTS. The broker holds two permanently open descriptors per +connected agent -- the agent's PUB connects to the XSUB frontend and its SUB to +the XPUB backend -- plus a third, transient one while the agent fetches its +settings over the REQ/ROUTER settings socket. With Linux's usual soft +RLIMIT_NOFILE of 1024 and the broker's own ~30 descriptors of overhead, that +walls a fleet in at roughly 495 agents, and libzmq reports the wall almost +invisibly: tcp_listener_t::accept() lists EMFILE/ENFILE among its non-fatal +errnos, so a refused agent produces only a ZMQ_EVENT_ACCEPT_FAILED that nobody +was listening for. + +The decision logic (plan_fd_limit) is deliberately pure -- it takes the +observed limits as data and makes no syscalls -- so every branch, including the +platforms the build is not currently running on, is unit-testable from +tests/test_fd_limit.cpp. Mirrors the "pure evaluator" split that +src/doctor_checks.hpp documents. +*/ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 +#include +#include +#endif + +#ifdef __APPLE__ +#include +#include +#endif + +namespace Mads::detail { + +/// Descriptors a single connected agent costs the broker for as long as it +/// stays connected: one for its publisher, one for its subscriber. (The +/// settings request needs a third, but only while it is in flight.) +inline constexpr uint64_t FD_PER_AGENT = 2; + +/// The broker's own descriptor footprint, independent of fleet size: three +/// listening sockets, ~10 libzmq socket mailboxes, the context reaper's and +/// each I/O thread's eventfd/epoll pair, stdio, and the settings-file inotify +/// watch. Rounded up, so the capacity estimate errs on the safe side. +inline constexpr uint64_t FD_BROKER_OVERHEAD = 32; + +/// Soft limit at or below which raising is worth suggesting -- the default on +/// essentially every Linux distribution, and the value systemd hands a unit +/// that does not set LimitNOFILE=. +inline constexpr uint64_t FD_LOW_WATERMARK = 1024; + +/// Last-resort ceiling when the OS reports an unbounded hard limit and no +/// kernel cap could be read. Matches Linux's own default fs.nr_open. +inline constexpr uint64_t FD_ABSOLUTE_CEILING = 1048576; + +/// How many agents a given soft limit leaves room for. +inline uint64_t agent_capacity(uint64_t soft) { + if (soft <= FD_BROKER_OVERHEAD) + return 0; + return (soft - FD_BROKER_OVERHEAD) / FD_PER_AGENT; +} + +/// The descriptor limits in force for this process. `supported` is false where +/// the concept does not apply (Windows), in which case soft/hard are 0 and +/// nothing should be reported as a limit. +struct FdLimits { + bool supported = false; + uint64_t soft = 0; + /// The ceiling this process may raise `soft` to without privileges. Already + /// clamped to the kernel's own per-process cap (fs.nr_open on Linux, + /// kern.maxfilesperproc on macOS), so a plan built from it never proposes a + /// target that setrlimit() would reject. + uint64_t hard = 0; +}; + +namespace fd_limit_impl { + +/// Reads a single unsigned integer out of a /proc or /sys file. +inline std::optional read_uint_file(const char *path) { + std::ifstream in(path); + if (!in) + return std::nullopt; + uint64_t value = 0; + if (!(in >> value)) + return std::nullopt; + return value; +} + +/// The kernel's own hard ceiling on a per-process descriptor table, which is +/// NOT the same thing as RLIMIT_NOFILE's rlim_max: on macOS rlim_max is +/// routinely RLIM_INFINITY while setrlimit() still fails with EINVAL above +/// kern.maxfilesperproc, and on Linux it fails with EPERM above fs.nr_open. +/// Clamping here is what keeps plan_fd_limit() from proposing the impossible. +inline std::optional kernel_fd_ceiling() { +#if defined(__APPLE__) + int value = 0; + size_t size = sizeof(value); + if (sysctlbyname("kern.maxfilesperproc", &value, &size, nullptr, 0) == 0 && + value > 0) { + return static_cast(value); + } + return std::nullopt; +#elif defined(__linux__) + return read_uint_file("/proc/sys/fs/nr_open"); +#else + return std::nullopt; +#endif +} + +} // namespace fd_limit_impl + +/// Reads the limits currently in force, with `hard` clamped as described above. +inline FdLimits query_fd_limits() { + FdLimits limits; +#ifdef _WIN32 + // Windows has no RLIMIT_NOFILE. libzmq uses SOCKET handles, which are not C + // runtime descriptors and are bounded by the process handle quota rather + // than a settable per-process table; _setmaxstdio() governs only stdio + // FILE* streams and would do nothing for sockets. Reporting this as + // unsupported is honest; silently pretending to apply a limit would not be. + limits.supported = false; +#else + rlimit rl{}; + if (getrlimit(RLIMIT_NOFILE, &rl) != 0) + return limits; + limits.supported = true; + limits.soft = static_cast(rl.rlim_cur); + limits.hard = rl.rlim_max == RLIM_INFINITY + ? FD_ABSOLUTE_CEILING + : static_cast(rl.rlim_max); + if (const auto ceiling = fd_limit_impl::kernel_fd_ceiling()) + limits.hard = std::min(limits.hard, *ceiling); + // A hard limit below the soft one is nonsensical, but clamping keeps every + // downstream comparison well-behaved rather than underflowing. + limits.hard = std::max(limits.hard, limits.soft); +#endif + return limits; +} + +/// What should be done about the limit, and what to tell the operator. Pure: +/// built from `requested` plus the observed limits, with no syscalls. +struct FdLimitPlan { + enum class Action { + NotSupported, ///< no per-process descriptor limit on this platform + Unset, ///< nothing configured; report the limit and leave it alone + Invalid, ///< configured value makes no sense; ignored, limit untouched + AlreadyEnough,///< the soft limit already meets the request + Raise, ///< raise the soft limit to `target` + Clamped ///< raise it, but only as far as the OS allows + }; + + Action action = Action::Unset; + /// The soft limit that should end up in force. + uint64_t target = 0; + /// One line, ready to print, explaining the limit and what was done to it. + std::string message; + /// True when `message` reports something the operator should act on. + bool warn = false; +}; + +/// Renders "1024 soft / 1048576 hard, about 496 agents". +inline std::string describe_fd_limits(const FdLimits &limits) { + return std::to_string(limits.soft) + " soft / " + std::to_string(limits.hard) + + " hard, about " + std::to_string(agent_capacity(limits.soft)) + + " agents"; +} + +/// The hint appended to every message that reports a limit worth raising. +inline std::string fd_limit_hint() { + return "set `max_open_files` in the [broker] section of the settings file " + "(or LimitNOFILE= in the systemd unit) to raise it"; +} + +/// Decides what to do. `requested` is the resolved `max_open_files` value: +/// nullopt leaves the limit untouched, 0 means "as high as this process is +/// allowed to go", and a positive value is a specific soft limit. +inline FdLimitPlan plan_fd_limit(std::optional requested, + const FdLimits &limits) { + FdLimitPlan plan; + + if (!limits.supported) { + plan.action = FdLimitPlan::Action::NotSupported; + // Only worth saying anything when the operator actually asked for a limit + // and is entitled to know it did not take effect. + if (requested.has_value()) { + plan.message = "max_open_files is not applicable on this platform: it " + "has no per-process descriptor limit for sockets"; + plan.warn = true; + } + return plan; + } + + plan.target = limits.soft; + + if (!requested.has_value()) { + plan.action = FdLimitPlan::Action::Unset; + plan.message = "File descriptors: " + describe_fd_limits(limits); + // Nagging is only useful where raising would actually buy something. + if (limits.soft <= FD_LOW_WATERMARK && limits.hard > limits.soft) { + plan.warn = true; + plan.message += " -- " + fd_limit_hint(); + } + return plan; + } + + if (*requested < 0) { + // Same posture as [broker] io_threads: a bad value in the settings file + // must not stop the broker from starting. + plan.action = FdLimitPlan::Action::Invalid; + plan.warn = true; + plan.message = "Invalid [broker] max_open_files = " + + std::to_string(*requested) + ", ignoring it. File " + "descriptors: " + describe_fd_limits(limits); + return plan; + } + + // 0 means "give me everything this process is permitted to have". + uint64_t wanted = *requested == 0 ? limits.hard + : static_cast(*requested); + + if (wanted > limits.hard) { + plan.action = FdLimitPlan::Action::Clamped; + plan.target = limits.hard; + plan.warn = true; + plan.message = "File descriptors: max_open_files = " + + std::to_string(*requested) + + " exceeds this process's hard limit, clamping to " + + std::to_string(limits.hard) + " (about " + + std::to_string(agent_capacity(limits.hard)) + " agents). " + "Raising the hard limit needs LimitNOFILE= in the systemd " + "unit or a privileged `ulimit -Hn`"; + // Nothing to do if the clamped target is what we already have. + if (plan.target <= limits.soft) { + plan.action = FdLimitPlan::Action::AlreadyEnough; + plan.target = limits.soft; + } + return plan; + } + + if (wanted <= limits.soft) { + plan.action = FdLimitPlan::Action::AlreadyEnough; + plan.target = limits.soft; + plan.message = "File descriptors: " + describe_fd_limits(limits) + + " (already at or above max_open_files = " + + std::to_string(*requested) + ")"; + return plan; + } + + plan.action = FdLimitPlan::Action::Raise; + plan.target = wanted; + plan.message = "File descriptors: raising soft limit " + + std::to_string(limits.soft) + " -> " + + std::to_string(wanted) + " (hard " + + std::to_string(limits.hard) + "), about " + + std::to_string(agent_capacity(wanted)) + " agents"; + return plan; +} + +/// The result of acting on a plan. +struct FdLimitOutcome { + FdLimits limits; ///< the limits in force afterwards + bool applied = false; ///< a setrlimit() call was made and succeeded + std::string error; ///< why it failed, when it did +}; + +/// Carries out `plan`. Only ever raises the soft limit: rlim_max is left +/// untouched, since raising it needs CAP_SYS_RESOURCE and would simply fail +/// for an unprivileged broker, while raising the soft limit toward the hard +/// one never needs privileges at all. +inline FdLimitOutcome apply_fd_limit(const FdLimitPlan &plan) { + FdLimitOutcome outcome; + outcome.limits = query_fd_limits(); + + const bool wants_change = plan.action == FdLimitPlan::Action::Raise || + plan.action == FdLimitPlan::Action::Clamped; + if (!wants_change || !outcome.limits.supported) + return outcome; + +#ifndef _WIN32 + rlimit rl{}; + if (getrlimit(RLIMIT_NOFILE, &rl) != 0) { + outcome.error = std::strerror(errno); + return outcome; + } + rl.rlim_cur = static_cast(plan.target); + if (setrlimit(RLIMIT_NOFILE, &rl) != 0) { + outcome.error = std::strerror(errno); + return outcome; + } + outcome.applied = true; + outcome.limits = query_fd_limits(); +#endif + return outcome; +} + +/// True when `err` is the errno of a process (or system) descriptor table that +/// has filled up -- the condition every explanatory message here exists for. +inline bool is_fd_exhaustion(int err) { +#ifdef _WIN32 + // WSAEMFILE is what Winsock reports; EMFILE covers the CRT paths. + return err == EMFILE || err == 10024 /* WSAEMFILE */; +#else + return err == EMFILE || err == ENFILE; +#endif +} + +} // namespace Mads::detail diff --git a/src/doctor_checks.cpp b/src/doctor_checks.cpp index 0785e27..97ea55e 100644 --- a/src/doctor_checks.cpp +++ b/src/doctor_checks.cpp @@ -1,6 +1,7 @@ #include "doctor_checks.hpp" #include "broker_probe.hpp" +#include "detail/fd_limit.hpp" #include #include @@ -331,5 +332,66 @@ CheckResult check_curve_handshake(const std::string &uri, const CurveKeyCheck &c cfg.server_key_name, timeout)); } +/* ---- 8. Open-file limit --------------------------------------------------- */ + +CheckResult evaluate_fd_limit(bool supported, uint64_t soft, uint64_t hard, + std::optional configured) { + CheckResult r; + r.name = "Open-file limit"; + + if (!supported) { + r.status = Status::Pass; + r.message = "This platform has no per-process descriptor limit for " + "sockets, so fleet size is not bounded by one."; + return r; + } + + const uint64_t capacity = Mads::detail::agent_capacity(soft); + const std::string room = std::to_string(soft) + " descriptors, room for " + + "about " + std::to_string(capacity) + + " connected agents"; + + // A request the hard limit cannot satisfy is the one case where the settings + // file is actively misleading: it looks configured, but the broker will + // silently get less than it asked for. + if (configured.has_value() && *configured > 0 && + static_cast(*configured) > hard) { + r.status = Status::Warn; + r.message = "max_open_files = " + std::to_string(*configured) + + " exceeds this process's hard limit of " + + std::to_string(hard) + ", so the broker will get " + room + "."; + r.fix_hint = "Raise the hard limit with LimitNOFILE= in the systemd unit " + "(or `ulimit -Hn` as root); max_open_files cannot go above it."; + return r; + } + + if (soft > Mads::detail::FD_LOW_WATERMARK) { + r.status = Status::Pass; + r.message = "A broker started the same way as this check would have " + + room + "."; + return r; + } + + r.status = Status::Warn; + r.message = "A broker started the same way as this check would have only " + + room + ". Every connected agent costs two descriptors, and " + "libzmq refuses the ones past the limit almost silently."; + r.fix_hint = hard > soft + ? "Set `max_open_files` in the [broker] section of the " + "settings file (up to the hard limit of " + + std::to_string(hard) + + "), or LimitNOFILE= in the systemd unit." + : "Raise the hard limit with LimitNOFILE= in the systemd " + "unit, or `ulimit -Hn` as root -- the soft limit is " + "already at it."; + return r; +} + +CheckResult check_fd_limit(std::optional configured) { + const auto limits = Mads::detail::query_fd_limits(); + return evaluate_fd_limit(limits.supported, limits.soft, limits.hard, + configured); +} + } // namespace Doctor } // namespace Mads diff --git a/src/doctor_checks.hpp b/src/doctor_checks.hpp index a07d4d1..057578b 100644 --- a/src/doctor_checks.hpp +++ b/src/doctor_checks.hpp @@ -32,6 +32,7 @@ Author(s): Paolo Bosetti #include "broker_probe.hpp" #include +#include #include #include #include @@ -165,6 +166,24 @@ CheckResult evaluate_curve_handshake(const std::string &uri, CheckResult check_curve_handshake(const std::string &uri, const CurveKeyCheck &cfg, std::chrono::milliseconds timeout); +/* ---- 8. Open-file limit leaves room for the fleet ------------------------- + The broker holds two descriptors per connected agent -- the agent's + publisher on the XSUB frontend, its subscriber on the XPUB backend -- so + RLIMIT_NOFILE, not anything in libzmq, is what bounds fleet size. The + default soft limit of 1024 stops at roughly 495 agents, and libzmq refuses + everything past it near-silently. + + Reported from *this* process's limit: doctor cannot see what a broker + started under systemd would inherit, so the wording says "a broker started + the same way as this check". The evaluator takes the limits as plain + integers rather than the Mads::detail type that produces them, so this + installed header stays free of src/detail/. */ + +CheckResult evaluate_fd_limit(bool supported, uint64_t soft, uint64_t hard, + std::optional configured); + +CheckResult check_fd_limit(std::optional configured); + } // namespace Doctor } // namespace Mads diff --git a/src/main/broker.cpp b/src/main/broker.cpp index 4c2d82d..0848b8c 100644 --- a/src/main/broker.cpp +++ b/src/main/broker.cpp @@ -20,6 +20,7 @@ Author(s): Paolo Bosetti #include #include #endif +#include "../detail/fd_limit.hpp" #include "../detail/socket_options.hpp" #include "../detail/wire_format.hpp" #include "../exec_path.hpp" @@ -29,6 +30,7 @@ Author(s): Paolo Bosetti #include "../keypress.hpp" #include "../goback.hpp" #include "../service_discovery.hpp" +#include "../socket_monitor.hpp" #include #include #include @@ -332,6 +334,154 @@ class SubscriptionTable { zmq::socket_t _capture; }; +// Reports the one broker failure mode libzmq otherwise swallows entirely: an +// inbound agent connection that could not be accepted because the process has +// run out of file descriptors. +// +// libzmq's tcp_listener_t::accept() lists EMFILE and ENFILE among the errnos +// it treats as non-fatal, so it fires ZMQ_EVENT_ACCEPT_FAILED and returns -- +// and with no monitor attached, the refused agent produces no output at all. +// The broker looks healthy while silently turning every new agent away. Worse, +// the listening socket stays readable, so libzmq retries the failing accept as +// fast as it can poll; that is why everything here is rate-limited. +// +// The monitors subscribe to ZMQ_EVENT_ACCEPT_FAILED *only*. On an XPUB/XSUB +// pair carrying hundreds of agents, ZMQ_EVENT_ALL would deliver a per-peer +// event storm for no benefit. +class AcceptWatch { +public: + // Must be called before socket.bind(): SocketMonitor's ordering contract is + // that libzmq may fire (and an unattached monitor miss) events raised while + // the socket is coming up. + void watch(zmq::socket_t &socket, string const &address) { + auto entry = make_unique(); + entry->address = address; + entry->monitor.start(socket, ZMQ_EVENT_ACCEPT_FAILED); + _watched.push_back(std::move(entry)); + } + + void start_reporting(uint64_t fd_soft_limit, bool daemon) { + if (_watched.empty()) + return; + _running = true; + _thread = thread([this, fd_soft_limit, daemon]() { + report_loop(fd_soft_limit, daemon); + }); + } + + // Must run before the watched sockets are closed and before the context is + // terminated: SocketMonitor::stop() is what releases each monitor's inproc + // PAIR socket, and a leaked one makes zmq_ctx_term() block forever. + void stop() { + _running = false; + if (_thread.joinable()) + _thread.join(); + for (auto &w : _watched) + w->monitor.stop(); + _watched.clear(); + } + + ~AcceptWatch() { stop(); } + +private: + // SocketMonitor is non-copyable and non-movable, so the entries are held by + // pointer to keep the vector itself assignable. + struct Watched { + Mads::SocketMonitor monitor; + string address; + /// Failures already attributed to this endpoint. Only used to spot which + /// socket a new refusal came from, so the message names the one that just + /// turned an agent away rather than whichever failed first. + uint64_t seen = 0; + }; + + static constexpr auto SUMMARY_PERIOD = 10s; + static constexpr auto POLL_PERIOD = 250ms; + + void report_loop(uint64_t fd_soft_limit, bool daemon) { + uint64_t reported = 0; + // Latched separately per failure class. A refused connection is not always + // descriptor exhaustion -- ECONNABORTED, for a client that hangs up mid + // handshake, is routine -- and one of those must not consume the one-shot + // explanation that the descriptor-exhaustion case exists to deliver. + bool explained_exhaustion = false; + bool explained_other = false; + // Kept across polls: a refusal seen while the rollup timer has not expired + // still has to be attributable when the timer finally does. + string address; + int last_errno = 0; + auto next_summary = chrono::steady_clock::now() + SUMMARY_PERIOD; + + while (_running) { + this_thread::sleep_for(POLL_PERIOD); + + // The counters are monotonic, so summing them and remembering what has + // already been reported is all the bookkeeping a rollup needs. + uint64_t total = 0; + for (auto &w : _watched) { + const auto st = w->monitor.state(); + total += st.accept_failures; + if (st.accept_failures > w->seen) { + w->seen = st.accept_failures; + address = w->address; + last_errno = st.last_event_value; + } + } + if (total <= reported) + continue; + + const uint64_t fresh = total - reported; + const bool exhausted = Mads::detail::is_fd_exhaustion(last_errno); + bool &explained = exhausted ? explained_exhaustion : explained_other; + + if (!explained) { + explained = true; + reported = total; + next_summary = chrono::steady_clock::now() + SUMMARY_PERIOD; + if (exhausted) { + cerr << goback(1, !daemon) << fg::red << timestamp() + << "OUT OF FILE DESCRIPTORS: refused an agent connection on " + << address << "." << fg::reset << endl + << fg::yellow + << " The open-file limit is " << fd_soft_limit + << " and every connected agent needs " + << Mads::detail::FD_PER_AGENT + << " descriptors (publisher + subscriber), plus one more while " + "it fetches" + << endl + << " its settings -- room for about " + << Mads::detail::agent_capacity(fd_soft_limit) + << " agents. Set `max_open_files` in the [broker] section of " + "the settings" + << endl + << " file, or LimitNOFILE= in the systemd unit, to raise it." + << fg::reset << endl; + } else { + cerr << goback(1, !daemon) << fg::red << timestamp() + << "Refused an inbound connection on " << address << ": " + << std::strerror(last_errno) << fg::reset << endl; + } + continue; + } + + // Already explained once. Roll the rest up rather than let a listener + // that re-fires continuously scroll the explanation off the screen. + if (chrono::steady_clock::now() >= next_summary) { + cerr << goback(1, !daemon) << fg::red << timestamp() << fresh + << " more inbound connection(s) refused" + << (exhausted ? " (out of file descriptors)" : "") << fg::reset + << endl; + reported = total; + next_summary = chrono::steady_clock::now() + SUMMARY_PERIOD; + } + } + } + + vector> _watched; + thread _thread; + std::atomic _running{false}; +}; + // Install SIGINT/SIGTERM handlers that request a clean shutdown by stopping // the process-wide run flag. Used in daemon mode so a `kill`/`systemctl stop` (or CTRL-C) // unwinds the proxy and stops advertising instead of killing the process @@ -495,6 +645,36 @@ int main(int argc, char **argv) { auto socket_options = Mads::detail::SocketOptions::resolve(config["agents"], config[name]); + // The descriptor budget, settled before a single socket is bound so that the + // figure reported below is the one the proxy actually runs under. + // + // Fleet size is bounded by this limit, not by anything in libzmq: every + // connected agent holds two of the broker's descriptors for as long as it + // stays connected (its publisher on the XSUB frontend, its subscriber on the + // XPUB backend) and a third while it fetches its settings. A stock Linux + // soft limit of 1024 therefore walls a fleet in at roughly 495 agents -- and + // libzmq refuses everything past that almost silently, which is what + // AcceptWatch above exists to report. + // Read from the broker's own section only, like io_threads and + // settings_workers: this is a property of the broker process, not a + // socket option an agent could meaningfully inherit from [agents]. + const auto requested_fd_limit = + config[name]["max_open_files"].value(); + const auto fd_plan = Mads::detail::plan_fd_limit( + requested_fd_limit, Mads::detail::query_fd_limits()); + const auto fd_outcome = Mads::detail::apply_fd_limit(fd_plan); + if (!fd_plan.message.empty()) { + if (fd_plan.warn) + cerr << fg::yellow << fd_plan.message << fg::reset << endl; + else + cout << fd_plan.message << endl; + } + if (!fd_outcome.error.empty()) { + cerr << fg::red << "Could not raise the open-file limit: " + << fd_outcome.error << fg::reset << endl; + } + const uint64_t fd_soft_limit = fd_outcome.limits.soft; + // ZMQ_DEVELOPMENT.md ยง2.2: off by default. See SubscriptionTable's comment // above for the cost this opts into. const bool subscription_table_enabled = @@ -552,12 +732,17 @@ int main(int argc, char **argv) { } } + // Attached before the binds below, per SocketMonitor's ordering contract. + AcceptWatch accept_watch; + try { std::cout << "Binding broker frontend (XSUB) at " << style::bold << frontend_address << style::reset << endl; + accept_watch.watch(frontend, frontend_address); frontend.bind(frontend_address); std::cout << "Binding broker backend (XPUB) at " << style::bold << backend_address << style::reset << endl; + accept_watch.watch(backend, backend_address); backend.bind(backend_address); } catch (const zmq::error_t &e) { cerr << fg::red << "ZMQ error, could not connect: " << e.what() << fg::reset @@ -606,7 +791,9 @@ int main(int argc, char **argv) { if (crypto) curve_auth_ptr->setup_curve_server(settings_router, key_name); socket_options.apply(settings_router); + accept_watch.watch(settings_router, settings_address); settings_router.bind(settings_address); + accept_watch.start_reporting(fd_soft_limit, daemon); cout << "Binding broker shared settings (ROUTER, " << settings_workers << " worker" << (settings_workers == 1 ? "" : "s") << ") at " << style::bold << settings_address << style::reset << endl; @@ -851,6 +1038,10 @@ int main(int argc, char **argv) { settings_proxy_controller.close(); settings_proxy_controlled.close(); if (subscription_table) subscription_table->stop(); + // Before the sockets it monitors are closed and before the context is + // terminated: each monitor holds an inproc PAIR socket that would + // otherwise keep zmq_ctx_term() blocked forever. + accept_watch.stop(); frontend.close(); backend.close(); settings_router.close(); @@ -985,6 +1176,10 @@ int main(int argc, char **argv) { if (crypto) curve_auth_ptr = nullptr; if (subscription_table) subscription_table->stop(); + // Before the sockets it monitors are closed and before the context is + // terminated: each monitor holds an inproc PAIR socket that would + // otherwise keep zmq_ctx_term() blocked forever. + accept_watch.stop(); frontend.close(); backend.close(); settings_router.close(); diff --git a/src/main/doctor.cpp b/src/main/doctor.cpp index ccb3bf8..54940cc 100644 --- a/src/main/doctor.cpp +++ b/src/main/doctor.cpp @@ -631,6 +631,18 @@ int main(int argc, char *argv[]) { << style::reset << fg::reset << endl; } + // --- 8. open-file limit -------------------------------------------------- + // Last because it is the only check about the machine rather than the + // fleet's configuration -- and the one that explains a broker which passes + // every other check and still turns agents away past roughly 495 of them. + { + std::optional configured; + if (config.has_value()) { + configured = (*config)["broker"]["max_open_files"].value(); + } + print_result(Doctor::check_fd_limit(configured)); + } + cout << endl; if (overall_exit_code == 0) { cout << fg::green << style::bold << "All checks passed." << style::reset diff --git a/src/service_discovery.cpp b/src/service_discovery.cpp index 46146c2..c3e99ba 100644 --- a/src/service_discovery.cpp +++ b/src/service_discovery.cpp @@ -1,5 +1,7 @@ #include "service_discovery.hpp" +#include "detail/fd_limit.hpp" + #include #include #include @@ -106,6 +108,21 @@ bool socket_would_block() { #endif } +// Appended to any socket error caused by a full descriptor table. +// +// Without it, the broker's most common scaling failure reads as a bare +// "getifaddrs failed: Too many open files", which points the finger at service +// discovery rather than at the process-wide limit that is actually refusing +// agents. Discovery is only the messenger: the advertising loop is the sole +// thing in the broker that allocates a descriptor on a timer -- a netlink +// socket for getifaddrs(), then one UDP socket per interface -- so it is the +// first thing to fail and the only one that prints. +std::string fd_exhaustion_hint() { + return " (the process has run out of file descriptors -- raise the limit" + " with `max_open_files` in the settings file, or LimitNOFILE= in the" + " systemd unit)"; +} + std::string last_socket_error(const std::string &message) { #ifdef _WIN32 const DWORD error = WSAGetLastError(); @@ -127,9 +144,16 @@ std::string last_socket_error(const std::string &message) { details.back() == ' ')) { details.pop_back(); } - return message + ": " + details; + std::string result = message + ": " + details; + if (Mads::detail::is_fd_exhaustion(static_cast(error))) + result += fd_exhaustion_hint(); + return result; #else - return message + ": " + std::strerror(errno); + const int error = errno; + std::string result = message + ": " + std::strerror(error); + if (Mads::detail::is_fd_exhaustion(error)) + result += fd_exhaustion_hint(); + return result; #endif } @@ -951,6 +975,16 @@ ServiceDiscovery::list_broadcast_interfaces() const { } void ServiceDiscovery::advertising_loop() { + // Under descriptor exhaustion advertise_once() fails on every single tick. + // Printing each one would scroll away the messages that matter -- not least + // the broker's own report of the agents it is refusing -- so an unchanged + // failure is reported at most once per FAILURE_REPORT_PERIOD, with a count + // of what was suppressed. A *different* failure is always reported at once. + constexpr auto FAILURE_REPORT_PERIOD = std::chrono::seconds(30); + std::string last_failure; + uint64_t suppressed = 0; + auto next_failure_report = std::chrono::steady_clock::now(); + while (true) { ServiceInfo service; std::chrono::milliseconds interval{0}; @@ -966,9 +1000,23 @@ void ServiceDiscovery::advertising_loop() { try { advertise_once(service); + // Recovered: the next failure, even an identical one, is news again. + last_failure.clear(); + suppressed = 0; } catch (const std::exception &e) { - std::cerr << "ServiceDiscovery advertising failed: " << e.what() - << std::endl; + const std::string what = e.what(); + const auto now = std::chrono::steady_clock::now(); + if (what != last_failure || now >= next_failure_report) { + std::cerr << "ServiceDiscovery advertising failed: " << what; + if (what == last_failure && suppressed > 0) + std::cerr << " (and " << suppressed << " more like it)"; + std::cerr << std::endl; + last_failure = what; + suppressed = 0; + next_failure_report = now + FAILURE_REPORT_PERIOD; + } else { + ++suppressed; + } } std::unique_lock lock(_mutex); diff --git a/src/socket_monitor.cpp b/src/socket_monitor.cpp index 5ea5b53..22bc9e7 100644 --- a/src/socket_monitor.cpp +++ b/src/socket_monitor.cpp @@ -27,6 +27,8 @@ LinkEvent to_link_event(uint16_t zmq_event) { return LinkEvent::HandshakeFailedNoDetail; case ZMQ_EVENT_DISCONNECTED: return LinkEvent::Disconnected; + case ZMQ_EVENT_ACCEPT_FAILED: + return LinkEvent::AcceptFailed; default: return LinkEvent::None; } @@ -122,6 +124,14 @@ class SocketMonitor::Impl : public zmq::monitor_t { const char *addr) override { record(ev, addr); } + // libzmq's tcp_listener_t treats a failed accept() as non-fatal -- EMFILE + // and ENFILE are both in the errno list it tolerates -- so without this the + // agents a full descriptor table turns away are refused in complete + // silence. ev.value carries that errno. + void on_event_accept_failed(const zmq_event_t &ev, + const char *addr) override { + record(ev, addr); + } private: mutable std::mutex _mtx; @@ -137,6 +147,9 @@ class SocketMonitor::Impl : public zmq::monitor_t { _last_address = addr ? addr : ""; _state.last_event = _last_event; _state.last_event_address = _last_address; + _state.last_event_value = ev.value; + if (_last_event == LinkEvent::AcceptFailed) + ++_state.accept_failures; switch (_last_event) { case LinkEvent::HandshakeSucceeded: case LinkEvent::HandshakeFailedAuth: @@ -182,6 +195,13 @@ class SocketMonitor::Impl : public zmq::monitor_t { // In flight, neither up nor conclusively down. last_event still // records them for callers that want the finer detail. break; + case LinkEvent::AcceptFailed: + // Not a statement about any link: it is the listener reporting that a + // peer never became one. Scoring it as Down would mark a broker whose + // existing agents are all still connected as offline. The event and its + // errno stay in last_event/last_event_value, and accept_failures counts + // it; status is left exactly as it was. + break; } if (next == _state.status) return; // repeated retries collapse into the one transition diff --git a/src/socket_monitor.hpp b/src/socket_monitor.hpp index e301c78..9886875 100644 --- a/src/socket_monitor.hpp +++ b/src/socket_monitor.hpp @@ -38,6 +38,12 @@ enum class LinkEvent { HandshakeFailedProtocol, HandshakeFailedNoDetail, Disconnected, + /// A *bound* socket could not accept an inbound connection. Unlike every + /// other event here this describes the listener, not a link: the peer it + /// would have belonged to never got far enough to have one. The errno + /// libzmq reported (EMFILE when the process is out of file descriptors) + /// arrives with it, in LinkState::last_event_value. + AcceptFailed, }; /// Whether the link is usable *right now*. Where LinkEvent is a @@ -77,6 +83,16 @@ struct LinkState { /// When `status` last changed; empty while it is still Unknown. Callers /// report "down for 12s" from this. std::optional changed_at; + /// The integer libzmq attached to the most recent event. Its meaning is + /// per-event and it is 0 for the ones that carry nothing useful; the case + /// this exists for is AcceptFailed, where it is the errno accept(2) + /// failed with. + int last_event_value = 0; + /// Inbound connections this listener has failed to accept. Monotonic, so a + /// caller can report only what is new since it last looked -- which matters + /// because a full descriptor table leaves the listening socket permanently + /// readable, and libzmq re-fires the failure as fast as it can poll. + uint64_t accept_failures = 0; }; /** diff --git a/tests/test_doctor_checks.cpp b/tests/test_doctor_checks.cpp index 23afe7c..a255b1e 100644 --- a/tests/test_doctor_checks.cpp +++ b/tests/test_doctor_checks.cpp @@ -402,3 +402,54 @@ TEST_CASE("check_port_available passes when nothing is listening", "[doctor][por REQUIRE(r.status == Status::Pass); REQUIRE(r.message.find("is free") != std::string::npos); } + +/* ---- 8. Open-file limit -------------------------------------------------- + Pure evaluator, so the whole matrix is reachable without touching this + process's real rlimits -- including the Windows "no such limit" branch. */ + +TEST_CASE("evaluate_fd_limit passes where the platform has no such limit", + "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(false, 0, 0, std::nullopt); + REQUIRE(r.status == Status::Pass); + REQUIRE(r.fix_hint.empty()); +} + +TEST_CASE("evaluate_fd_limit warns at the stock 1024 soft limit", + "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(true, 1024, 1048576, std::nullopt); + REQUIRE(r.status == Status::Warn); + // The agent count is what connects the limit to the symptom an operator + // actually sees -- agents refused past roughly 495 of them. + REQUIRE(r.message.find("496") != std::string::npos); + REQUIRE(r.fix_hint.find("max_open_files") != std::string::npos); +} + +TEST_CASE("evaluate_fd_limit points at the hard limit when the soft one is " + "already there", + "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(true, 1024, 1024, std::nullopt); + REQUIRE(r.status == Status::Warn); + // max_open_files cannot help here; only raising the hard limit can. + REQUIRE(r.fix_hint.find("LimitNOFILE") != std::string::npos); + REQUIRE(r.fix_hint.find("max_open_files") == std::string::npos); +} + +TEST_CASE("evaluate_fd_limit passes with a raised limit", "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(true, 65536, 1048576, std::nullopt); + REQUIRE(r.status == Status::Pass); + REQUIRE(r.fix_hint.empty()); +} + +TEST_CASE("evaluate_fd_limit warns when max_open_files exceeds the hard limit", + "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(true, 65536, 65536, 200000); + REQUIRE(r.status == Status::Warn); + REQUIRE(r.message.find("exceeds") != std::string::npos); + REQUIRE(r.fix_hint.find("LimitNOFILE") != std::string::npos); +} + +TEST_CASE("evaluate_fd_limit ignores a configured value the limit can satisfy", + "[doctor][fd_limit]") { + auto r = Mads::Doctor::evaluate_fd_limit(true, 65536, 1048576, 65536); + REQUIRE(r.status == Status::Pass); +} diff --git a/tests/test_fd_limit.cpp b/tests/test_fd_limit.cpp new file mode 100644 index 0000000..001068f --- /dev/null +++ b/tests/test_fd_limit.cpp @@ -0,0 +1,207 @@ +// Pins Mads::detail's descriptor-limit logic (src/detail/fd_limit.hpp), which +// backs the `[broker] max_open_files` setting. +// +// plan_fd_limit() is pure -- it takes the observed limits as data and makes no +// syscalls -- so every branch is exercised here on every platform, including +// the ones the build is not currently running on (a Windows "not applicable" +// plan is asserted from Linux, and vice versa). +// +// Port range for this file: 44400-44449 (none of these cases bind a socket, +// but the range is reserved for consistency with the rest of the suite). +#include + +#include + +#include "detail/fd_limit.hpp" + +using namespace Mads::detail; + +namespace { + +FdLimits limits(uint64_t soft, uint64_t hard, bool supported = true) { + FdLimits l; + l.supported = supported; + l.soft = soft; + l.hard = hard; + return l; +} + +bool contains(const std::string &haystack, const std::string &needle) { + return haystack.find(needle) != std::string::npos; +} + +} // namespace + +/* ---- capacity arithmetic ------------------------------------------------ */ + +TEST_CASE("agent_capacity: two descriptors per agent above the broker's own", + "[fd_limit]") { + // The wall this whole feature exists for: a stock 1024 soft limit. + REQUIRE(agent_capacity(1024) == (1024 - FD_BROKER_OVERHEAD) / 2); + REQUIRE(agent_capacity(1024) == 496); + REQUIRE(agent_capacity(65536) == (65536 - FD_BROKER_OVERHEAD) / 2); +} + +TEST_CASE("agent_capacity: a limit below the broker's own footprint is zero", + "[fd_limit]") { + REQUIRE(agent_capacity(FD_BROKER_OVERHEAD) == 0); + REQUIRE(agent_capacity(0) == 0); + REQUIRE(agent_capacity(8) == 0); +} + +/* ---- platforms without a descriptor limit ------------------------------- */ + +TEST_CASE("plan_fd_limit: unsupported platform says nothing when unconfigured", + "[fd_limit]") { + const auto plan = plan_fd_limit(std::nullopt, limits(0, 0, false)); + REQUIRE(plan.action == FdLimitPlan::Action::NotSupported); + REQUIRE_FALSE(plan.warn); + REQUIRE(plan.message.empty()); +} + +TEST_CASE("plan_fd_limit: unsupported platform warns that a request had no " + "effect", + "[fd_limit]") { + const auto plan = plan_fd_limit(65536, limits(0, 0, false)); + REQUIRE(plan.action == FdLimitPlan::Action::NotSupported); + REQUIRE(plan.warn); + REQUIRE(contains(plan.message, "not applicable")); +} + +/* ---- unconfigured: report, and nag only when raising would help ---------- */ + +TEST_CASE("plan_fd_limit: unconfigured reports the limit without touching it", + "[fd_limit]") { + const auto plan = plan_fd_limit(std::nullopt, limits(65536, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Unset); + REQUIRE(plan.target == 65536); + REQUIRE_FALSE(plan.warn); + REQUIRE(contains(plan.message, "65536 soft")); +} + +TEST_CASE("plan_fd_limit: unconfigured warns at the stock 1024 soft limit", + "[fd_limit]") { + const auto plan = plan_fd_limit(std::nullopt, limits(1024, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Unset); + REQUIRE(plan.warn); + REQUIRE(contains(plan.message, "max_open_files")); + // The capacity estimate is the whole point of the line: it is what turns + // "1024" into "this is why the 461st agent was refused". + REQUIRE(contains(plan.message, "496 agents")); +} + +TEST_CASE("plan_fd_limit: no nagging when the hard limit leaves nothing to gain", + "[fd_limit]") { + const auto plan = plan_fd_limit(std::nullopt, limits(1024, 1024)); + REQUIRE(plan.action == FdLimitPlan::Action::Unset); + REQUIRE_FALSE(plan.warn); +} + +/* ---- bad values are ignored, never fatal -------------------------------- */ + +TEST_CASE("plan_fd_limit: a negative request is ignored with a warning", + "[fd_limit]") { + const auto plan = plan_fd_limit(-1, limits(1024, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Invalid); + REQUIRE(plan.warn); + REQUIRE(plan.target == 1024); // untouched + REQUIRE(contains(plan.message, "Invalid")); +} + +/* ---- zero means "as high as this process may go" ------------------------ */ + +TEST_CASE("plan_fd_limit: zero targets the hard limit", "[fd_limit]") { + const auto plan = plan_fd_limit(0, limits(1024, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Raise); + REQUIRE(plan.target == 1048576); +} + +TEST_CASE("plan_fd_limit: zero is a no-op when soft already equals hard", + "[fd_limit]") { + const auto plan = plan_fd_limit(0, limits(4096, 4096)); + REQUIRE(plan.action == FdLimitPlan::Action::AlreadyEnough); + REQUIRE(plan.target == 4096); +} + +/* ---- explicit targets --------------------------------------------------- */ + +TEST_CASE("plan_fd_limit: a request below the current soft limit is a no-op", + "[fd_limit]") { + const auto plan = plan_fd_limit(512, limits(4096, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::AlreadyEnough); + REQUIRE(plan.target == 4096); // never lowers an existing limit +} + +TEST_CASE("plan_fd_limit: a request equal to the soft limit is a no-op", + "[fd_limit]") { + const auto plan = plan_fd_limit(4096, limits(4096, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::AlreadyEnough); + REQUIRE(plan.target == 4096); +} + +TEST_CASE("plan_fd_limit: a reachable request raises the soft limit", + "[fd_limit]") { + const auto plan = plan_fd_limit(65536, limits(1024, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Raise); + REQUIRE(plan.target == 65536); + REQUIRE_FALSE(plan.warn); + REQUIRE(contains(plan.message, "1024 -> 65536")); +} + +TEST_CASE("plan_fd_limit: a request beyond the hard limit is clamped, not " + "refused", + "[fd_limit]") { + const auto plan = plan_fd_limit(2000000, limits(1024, 1048576)); + REQUIRE(plan.action == FdLimitPlan::Action::Clamped); + REQUIRE(plan.target == 1048576); + REQUIRE(plan.warn); + // The operator needs to know the remedy is outside the settings file. + REQUIRE(contains(plan.message, "LimitNOFILE")); +} + +TEST_CASE("plan_fd_limit: a clamp that lands on the current limit becomes a " + "no-op but still warns", + "[fd_limit]") { + const auto plan = plan_fd_limit(65536, limits(1024, 1024)); + REQUIRE(plan.action == FdLimitPlan::Action::AlreadyEnough); + REQUIRE(plan.target == 1024); + REQUIRE(plan.warn); // the request could not be honoured; say so +} + +/* ---- rendering ---------------------------------------------------------- */ + +TEST_CASE("describe_fd_limits: soft, hard and the implied agent count", + "[fd_limit]") { + const auto text = describe_fd_limits(limits(1024, 1048576)); + REQUIRE(contains(text, "1024 soft")); + REQUIRE(contains(text, "1048576 hard")); + REQUIRE(contains(text, "496 agents")); +} + +/* ---- exhaustion errnos -------------------------------------------------- */ + +TEST_CASE("is_fd_exhaustion: recognises a full descriptor table", "[fd_limit]") { + REQUIRE(is_fd_exhaustion(EMFILE)); +#ifndef _WIN32 + REQUIRE(is_fd_exhaustion(ENFILE)); +#endif + REQUIRE_FALSE(is_fd_exhaustion(ECONNREFUSED)); + REQUIRE_FALSE(is_fd_exhaustion(0)); +} + +/* ---- the live query ----------------------------------------------------- */ + +TEST_CASE("query_fd_limits: reports a sane, self-consistent limit", + "[fd_limit]") { + const auto live = query_fd_limits(); +#ifdef _WIN32 + REQUIRE_FALSE(live.supported); +#else + REQUIRE(live.supported); + REQUIRE(live.soft > 0); + // query_fd_limits() clamps hard to the kernel's per-process ceiling and then + // to at least soft, so this must hold however exotic the host's rlimits are. + REQUIRE(live.hard >= live.soft); + REQUIRE(live.hard <= FD_ABSOLUTE_CEILING); +#endif +}