Skip to content

fix: raise fd limit at startup and fail fast when hard limit is too low#7

Merged
janx merged 6 commits into
mainfrom
claude/issue-4-20260327-0810
Mar 27, 2026
Merged

fix: raise fd limit at startup and fail fast when hard limit is too low#7
janx merged 6 commits into
mainfrom
claude/issue-4-20260327-0810

Conversation

@janx

@janx janx commented Mar 27, 2026

Copy link
Copy Markdown
Owner

Fixes #4

Goal

  • Prevent "Too many open files" crash during bulk sync on macOS, including when the hard fd limit is also too low

Principle Alignment

  • Fail Fast, Fail Early: detect and reject insufficient fd limits before the indexer starts, with an actionable OS-specific fix message

Result

  • raise_fd_limit() tries to raise both hard and soft limits to 65536; if the hard-limit raise is denied (EPERM), falls back to raising soft to the current hard ceiling
  • check_fd_limit_for_indexer() fails fast with an actionable error if the effective limit is below 4096 after the raise attempt
  • Startup banner shows effective fd limit; yellow WARNING if below minimum
  • Re-sync required: no
  • What to do next: verify on macOS with a restrictive launchctl config

Generated with Claude Code

Copilot AI review requested due to automatic review settings March 27, 2026 08:51
Add GitHub Actions release workflow triggered by v* tags that builds
ckbadger binaries for Linux x86_64, macOS x86_64, and macOS aarch64.
Fix build.rs to handle detached HEAD (tag checkout in CI) by making
branch_name optional — detached HEAD now produces clean version strings
like main branch (e.g. 0.1.0@abcdef123456).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds startup-time file descriptor (fd) limit management to reduce “Too many open files” failures during RocksDB-heavy bulk sync, and to fail fast when the effective limit is insufficient.

Changes:

  • Introduces raise_fd_limit() (best-effort) and check_fd_limit_for_indexer() (fail-fast below a minimum threshold).
  • Displays the effective fd limit in the startup system line (with conditional warning formatting).
  • Adds unit tests covering fd-limit helpers and updates existing print_startup_info call sites.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/cli/src/main.rs Outdated
Comment thread crates/cli/src/main.rs
Comment thread crates/cli/src/main.rs Outdated
When a SporeCluster ID was passed to GET /spore/objects/{id}, the API
returned it as a spore item because both clusters and spore items share
CF_SPORE_DATA. The frontend then rendered the cluster as a spore detail
page and fetched item activities — which are always empty for clusters
(no outpoints are recorded for clusters).

Now get_spore and list_spore_item_activities reject cluster entries with
404, causing the frontend to fall through to the collection view.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@janx

janx commented Mar 27, 2026

Copy link
Copy Markdown
Owner Author

@claude fix failed checks

@claude

claude Bot commented Mar 27, 2026

Copy link
Copy Markdown

Claude finished @janx's task in 1m 53s —— View job


Fixed failed checks

  • Check CI status to identify failing checks — cargo fmt check failed in "Rust Check & Lint"
  • Read the changed file to understand the current code
  • Fix the formatting issue — each format arg in check_fd_limit_for_indexer now on its own line (matching what rustfmt expected)
  • Push fix

Root cause: rustfmt requires each argument in a long format macro call to be on its own line. The args fd_limit, FD_LIMIT_MIN, FD_LIMIT_TARGET, FD_LIMIT_TARGET, FD_LIMIT_TARGET, FD_LIMIT_TARGET were packed two-per-line; the fix puts one per line (crates/cli/src/main.rs:135-140).
· Branch

@janx

janx commented Mar 27, 2026

Copy link
Copy Markdown
Owner Author

@claude check copilot review comments

@claude

claude Bot commented Mar 27, 2026

Copy link
Copy Markdown

Claude finished @janx's task in 3m 57s —— View job


Fixed Copilot review comments

  • Read crates/cli/src/main.rs to understand current code
  • Comment 1 (line 364): services.contains(&"indexer".to_string())services.iter().any(|s| s == "indexer") — avoids String allocation
  • Comment 2 (line 61): silent return 0 on getrlimit failure → raise_fd_limit() now returns Result<u64> and bails with the OS error, so users see "getrlimit(RLIMIT_NOFILE) failed: …" instead of "fd limit 0 is too low"
  • Comment 3 (line 773): WARNING banner threshold changed from fd_limit < FD_LIMIT_MIN (unreachable when indexer runs, since we bail! before print_startup_info) to fd_limit < FD_LIMIT_TARGET — now shows a useful yellow warning when the limit was raised but couldn't reach 65536 (e.g., hard limit capped at 8192)
  • Update two raise_fd_limit tests to .expect(...) the new Result
  • Pushed as commit e860177
    · Branch

github-actions Bot and others added 3 commits March 27, 2026 17:54
If the process hard fd limit is also low (e.g. macOS with a locked-down
launchctl config), the previous soft-limit raise would silently fall back
to the hard ceiling and proceed, only to crash mid bulk-sync when RocksDB
exhausted the available file descriptors.

Changes:
- raise_fd_limit() now tries to raise BOTH rlim_max and rlim_cur to 65536;
  falls back to raising soft limit to current hard ceiling if the hard raise
  is denied (requires root on Linux, may work for regular users on macOS)
- check_fd_limit_for_indexer() fails fast with an actionable error (including
  OS-specific fix commands) if the effective limit is below FD_LIMIT_MIN=4096
  after the raise attempt -- aligns with Fail Fast, Fail Early principle
- cmd_run() calls check_fd_limit_for_indexer() before spawning the supervisor
  when the indexer service is selected
- cmd_internal(Indexer) adds the same check as a safety net for direct invocation
- print_startup_info() now shows the fd limit in the System line; shows a
  WARNING in yellow if below FD_LIMIT_MIN
- Add tests: raise_fd_limit nonzero, idempotent; check_fd_limit passes/fails

Co-authored-by: busyforking <janx@users.noreply.github.com>
…ndexer

Co-authored-by: busyforking <janx@users.noreply.github.com>
- raise_fd_limit() now returns Result<u64>; bail! with OS error on
  getrlimit failure instead of silently returning 0 (misleading message)
- services.iter().any(|s| s == "indexer") avoids String allocation
- print_startup_info warning threshold changed to FD_LIMIT_TARGET so
  the banner shows useful signal when limit was raised but below target

Co-authored-by: busyforking <janx@users.noreply.github.com>
@janx janx force-pushed the claude/issue-4-20260327-0810 branch from e860177 to 927c288 Compare March 27, 2026 09:55
rlim_t is u64 on Linux x86_64 but u32 on some other platforms,
so the casts are needed for portability.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@janx janx merged commit 95d9f82 into main Mar 27, 2026
7 checks passed
@janx janx deleted the claude/issue-4-20260327-0810 branch March 27, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS: indexer crashes with "Too many open files" during bulk sync

2 participants