Skip to content

fix: increase connection backlog based on max connections#7034

Merged
LesnyRumcajs merged 1 commit into
mainfrom
set-connection-backlog
May 11, 2026
Merged

fix: increase connection backlog based on max connections#7034
LesnyRumcajs merged 1 commit into
mainfrom
set-connection-backlog

Conversation

@LesnyRumcajs
Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs commented May 11, 2026

Summary of changes

Changes introduced in this pull request:

  • increases the connection backlog based on what we set via FOREST_RPC_MAX_CONNECTIONS.
# main
❯ OHA_CONCURRENCY=1000 bash rpc_bench.sh
method:    eth_chainId
params:    []
payload:   {"jsonrpc":"2.0","method":"eth_chainId","params":[],"id":1}
lotus:     http://127.0.0.1:1234/rpc/v1
forest:    http://127.0.0.1:2345/rpc/v1

=== sanity probe ===
forest   -> {"jsonrpc":"2.0","id":1,"result":"0x4cb2f"}

=== oha ===
--- forest (http://127.0.0.1:2345/rpc/v1) ---
warmup (200 requests)...
Summary:
  Success rate:	100.00%
  Total:	1.0690 sec
  Slowest:	1.0450 sec
  Fastest:	0.0001 sec
  Average:	0.1439 sec
  Requests/sec:	1870.8967

  Total data:	83.98 KiB
  Size/request:	43 B
  Size/sec:	78.56 KiB
# PR
❯ OHA_CONCURRENCY=1000 bash rpc_bench.sh
method:    eth_chainId
params:    []
payload:   {"jsonrpc":"2.0","method":"eth_chainId","params":[],"id":1}
lotus:     http://127.0.0.1:1234/rpc/v1
forest:    http://127.0.0.1:2345/rpc/v1

=== sanity probe ===
forest   -> {"jsonrpc":"2.0","id":1,"result":"0x4cb2f"}

=== oha ===
--- forest (http://127.0.0.1:2345/rpc/v1) ---
warmup (200 requests)...
Summary:
  Success rate:	100.00%
  Total:	44.1626 ms
  Slowest:	42.3777 ms
  Fastest:	0.0739 ms
  Average:	8.1844 ms
  Requests/sec:	45287.1774

  Total data:	83.98 KiB
  Size/request:	43 B
  Size/sec:	1.86 MiB

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

  • Improvements
    • Enhanced error handling and reporting for network listener initialization across metrics, healthcheck, and RPC endpoints
    • Increased robustness against connection spikes with optimized listen backlog configuration
    • RPC server maximum connections now configurable via environment variable with default fallback to 1000

Review Change Stack

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner May 11, 2026 13:26
@LesnyRumcajs LesnyRumcajs requested review from akaladarshi and hanabi1224 and removed request for a team May 11, 2026 13:26
@LesnyRumcajs LesnyRumcajs added the RPC requires calibnet RPC checks to run on CI label May 11, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Walkthrough

This PR centralizes TCP listener creation across the daemon and offline server by introducing a configurable public default_max_connections() function in the RPC module and enhancing the shared bind_tcp_listener helper with improved error context and minimum backlog enforcement. All three daemon services and the offline server are updated to use this unified approach.

Changes

TCP Listener Binding Centralization

Layer / File(s) Summary
RPC Configuration Export
src/rpc/mod.rs
Introduces public default_max_connections() function reading FOREST_RPC_MAX_CONNECTIONS environment variable with default of 1000; removes internal DEFAULT_MAX_CONNECTIONS lazy static and updates RPC server config to call the new function.
Enhanced TCP Binding Helper
src/utils/net.rs
Adds MIN_LISTEN_BACKLOG: u32 = 1024 constant and updates bind_tcp_listener to enforce minimum backlog on socket listen and wrap socket creation/binding/listening with contextual error messages via anyhow::Context.
Daemon Services Refactoring
src/daemon/mod.rs
Metrics, healthcheck, and RPC services replace direct TcpListener::bind calls with shared bind_tcp_listener helper; RPC service passes default_max_connections() and propagates bind errors via ? instead of panicking.
Offline Server Refactoring
src/tool/offline_server/server.rs
Offline RPC server TCP listener creation uses shared bind_tcp_listener helper configured with default_max_connections().

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • ChainSafe/forest#6528: Introduced the original DEFAULT_MAX_CONNECTIONS lazy static in src/rpc/mod.rs that this PR replaces with a public default_max_connections() function.

Suggested reviewers

  • akaladarshi
  • sudo-shashank
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the primary objective of the changeset: increasing the TCP connection backlog based on the max connections configuration, which is the core improvement demonstrated by the benchmark results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch set-connection-backlog
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch set-connection-backlog

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/rpc/mod.rs`:
- Around line 466-472: The current default_max_connections() accepts 0 which
disables RPC slots; update the parsing pipeline for static VALUE in
default_max_connections to reject zero by filtering parsed u32 values (e.g., add
.filter(|&v| v > 0) after the .and_then(|it| it.parse().ok())) so that if the
env var parses to 0 it falls back to the unwrap_or(1000) default; keep the same
LazyLock VALUE and env var FOREST_RPC_MAX_CONNECTIONS but ensure the parsed
value is >= 1 before returning it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: bb4fbc2e-4aa8-4079-8f3b-be46199adb57

📥 Commits

Reviewing files that changed from the base of the PR and between 7bef5d7 and 59c026f.

📒 Files selected for processing (4)
  • src/daemon/mod.rs
  • src/rpc/mod.rs
  • src/tool/offline_server/server.rs
  • src/utils/net.rs

Comment thread src/rpc/mod.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 26.47059% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.19%. Comparing base (7bef5d7) to head (59c026f).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/utils/net.rs 0.00% 13 Missing ⚠️
src/daemon/mod.rs 0.00% 8 Missing ⚠️
src/tool/offline_server/server.rs 0.00% 3 Missing ⚠️
src/rpc/mod.rs 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/rpc/mod.rs 89.32% <90.00%> (+0.11%) ⬆️
src/tool/offline_server/server.rs 31.30% <0.00%> (-0.28%) ⬇️
src/daemon/mod.rs 28.11% <0.00%> (+0.09%) ⬆️
src/utils/net.rs 43.90% <0.00%> (-8.28%) ⬇️

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bef5d7...59c026f. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue May 11, 2026
Merged via the queue into main with commit 17e1991 May 11, 2026
35 of 58 checks passed
@LesnyRumcajs LesnyRumcajs deleted the set-connection-backlog branch May 11, 2026 15:25
@coderabbitai coderabbitai Bot mentioned this pull request May 12, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC requires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants