π fix: raise RLIMIT_NOFILE at serve startup β fd exhaustion silently wedges accept()#150
Conversation
flupkede
left a comment
There was a problem hiding this comment.
β Approve β well-motivated, production-tested, standard daemon practice.
The silent-wedge failure mode (axum 0.7 accept loop retrying EMFILE while the process looks healthy to its supervisor) is exactly the class of bug that ships to production undetected. Raising soft β hard RLIMIT_NOFILE at startup is what nginx/envoy/postgres all do; the macOS kern.maxfilesperproc clamp is the correct pre-emptive fix for setrlimit EINVAL.
Correctness verified:
- Call-site placement is right: after
auto_discover_from_cwd(repo count is final), beforeServeState::new(consumes config) and beforeTcpListener::bind. #[cfg(unix)]gating consistent at function def + call site.unsafeblock is sound (locally-ownedrlimitstruct, SAFETY comment present).#[cfg(target_os = "macos")]/#[cfg(not(...))]paired for sysctl clamp.- Non-fatal error handling (log + continue with inherited limit) β correct choice, never breaks startup.
lim.rlim_cur = beforerollback on setrlimit failure.info!/warn!already imported atsrc/serve/mod.rs:34, no new imports needed.libccorrectly added as[target.'cfg(unix)'.dependencies](not unconditional).
Minor nits (non-blocking, feel free to ignore):
CString::new("kern.maxfilesperproc").unwrap()β.expect("kernel sysctl name has no NUL")is slightly more idiomatic for a known-safe constant.FDS_PER_REPO_ESTIMATE = 20is macOS-measured; on Linux the per-repo fd cost may differ, so the warning could fire conservatively at high repo counts. Not wrong, just informational.- No unit test β acceptable for a platform-specific syscall with a documented manual repro in the PR body.
The follow-up idea (custom accept loop / periodic fd-headroom check that logs at ERROR on residual EMFILE) is a good candidate for a separate PR β agree it's out of scope here.
Will rebase + merge. Thanks for the thorough write-up and repro.
flupkede
left a comment
There was a problem hiding this comment.
β Approve β well-motivated, production-tested, standard daemon practice.
The silent-wedge failure mode (axum 0.7 accept loop retrying EMFILE while the process looks healthy to its supervisor) is exactly the class of bug that ships to production undetected. Raising soft β hard RLIMIT_NOFILE at startup is what nginx/envoy/postgres all do; the macOS kern.maxfilesperproc clamp is the correct pre-emptive fix for setrlimit EINVAL.
Correctness verified:
- Call-site placement is right: after
auto_discover_from_cwd(repo count is final), beforeServeState::new(consumes config) and beforeTcpListener::bind. #[cfg(unix)]gating consistent at function def + call site.unsafeblock is sound (locally-ownedrlimitstruct, SAFETY comment present).#[cfg(target_os = "macos")]/#[cfg(not(...))]paired for sysctl clamp.- Non-fatal error handling (log + continue with inherited limit) β correct choice, never breaks startup.
lim.rlim_cur = beforerollback on setrlimit failure.info!/warn!already imported atsrc/serve/mod.rs:34, no new imports needed.libccorrectly added as[target.'cfg(unix)'.dependencies](not unconditional).
Minor nits (non-blocking, feel free to ignore):
CString::new("kern.maxfilesperproc").unwrap()β.expect("kernel sysctl name has no NUL")is slightly more idiomatic for a known-safe constant.FDS_PER_REPO_ESTIMATE = 20is macOS-measured; on Linux the per-repo fd cost may differ, so the warning could fire conservatively at high repo counts. Not wrong, just informational.- No unit test β acceptable for a platform-specific syscall with a documented manual repro in the PR body.
The follow-up idea (custom accept loop / periodic fd-headroom check that logs at ERROR on residual EMFILE) is a good candidate for a separate PR β agree it's out of scope here.
Will rebase + merge. Thanks for the thorough write-up and repro.
β¦wedges accept() serve's fd demand scales with registered repo count (LMDB env + tantivy FTS segments + file-watcher handles β 15-20 fds per warm repo). Under process supervisors the default soft limit is often 256 (macOS launchd agents, some systemd/docker configs). Once the process saturates it: - tantivy logs 'Too many open files' (errno 24) warnings, and - accept(2) fails with EMFILE; axum's accept loop sleeps and retries silently, so the daemon looks alive to its supervisor while every new connection is refused or reset. No ERROR log, no exit β a silent wedge. Observed in production: 60 registered repos (~1000 fds needed) under a macOS LaunchAgent β serve answered for ~15s after start (until repo warmup consumed the fd budget), then reset every connection while the process stayed 'healthy', deterministically across restarts. Fix, at run_serve startup before any store open or bind: 1. Raise the RLIMIT_NOFILE soft limit to the hard limit (standard daemon practice β nginx/envoy/postgres do the same). On macOS the target is clamped to kern.maxfilesperproc so setrlimit cannot fail with EINVAL. Failures are non-fatal and logged. 2. Log the raise at INFO. 3. If the effective limit still looks too small for the registered repo count (repos Γ 20 + 256 headroom), emit a loud actionable WARN naming the supervisor knobs (launchd SoftResourceLimits.NumberOfFiles, systemd LimitNOFILE, ulimit -n). Verified at scale: with ulimit -n 256 and 60 registered repos, an unpatched serve saturates at 255/256 fds (EMFILE in logs, wedge under launchd); the patched serve logs 'Raised RLIMIT_NOFILE soft limit 256 β 61440', runs at ~300 fds, and answers MCP handshakes indefinitely. cargo clippy -D warnings clean; cargo test --lib --bins green (579 + 575).
b73d5db to
07edae5
Compare
The bug
codesearch serve's file-descriptor demand scales with registered repo count β each warm repo holds an LMDB env + tantivy FTS segment handles + file-watcher handles (β 15β20 fds measured on macOS). Under process supervisors the default softRLIMIT_NOFILEis often 256 (macOS launchd agents, some systemd/docker configs). Once repo warmup saturates that limit:Too many open files(errno 24) warnings, andaccept(2)fails withEMFILE; axum 0.7's accept loop sleeps and retries silently β the daemon looks alive to its supervisor (KeepAlive sees a healthy pid) while every new connection is refused or reset. No ERROR log, no exit. A silent wedge.How we hit it
Production workspace with 60 registered repos (~1000 fds needed) running serve as a macOS LaunchAgent: serve answered MCP handshakes for ~15s after every start (until warmup consumed the fd budget), then reset every connection while the process stayed 'healthy' β deterministic across restarts. Diagnosis artifacts: errno-24 lines in
serve.log,sampleshowing all threads idle-parked with the listener effectively dead, process fd table saturated at exactly 255/256 under aulimit -n 256repro.The fix
At
run_servestartup, before any store open or listener bind (unix only, no-op elsewhere):RLIMIT_NOFILEto the hard limit β standard daemon practice (nginx/envoy/postgres all do it). On macOS the target is clamped tokern.maxfilesperprocsosetrlimitcannot fail withEINVAL. Failures are non-fatal and logged.Raised RLIMIT_NOFILE soft limit 256 β 61440.repos Γ 20 + 256headroom), emit a loud actionable WARN naming the supervisor knobs (launchdSoftResourceLimits.NumberOfFiles, systemdLimitNOFILE,ulimit -n) β so the failure mode becomes an explicit operator decision instead of a silent wedge.Adds
libcas a directcfg(unix)dependency (already in the tree transitively).Verification
ulimit -n 256+ 60 registered repos β unpatched serve saturates at 255/256 fds (EMFILE in logs; wedges under launchd); patched serve logs the raise, runs at ~300 fds, and answers MCP handshakes indefinitely under the same conditions.cargo clippy --lib --bins -- -D warningsclean.cargo test --lib --binsgreen (579 + 575 pass, 13 ignored β same as develop).Possible follow-up (not in this PR)
Even with the raise, a truly exhausted process still wedges silently because axum swallows accept errors. A custom accept loop (or a periodic fd-headroom check) that logs at ERROR when
accepthitsEMFILE/ENFILEwould make the residual case loud too. Happy to take a stab if you want it.