feat: limit Redis server connections - #3498
Open
thweetkomputer wants to merge 3 commits into
Open
Conversation
thweetkomputer
marked this pull request as ready for review
August 28, 2026 04:27
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a server-side admission limit for Redis-only listeners to prevent idle clients (and, for SSL, expensive TLS handshakes) from consuming connection resources before request-level concurrency limits apply.
Changes:
- Introduces
ServerOptions.redis_max_connectionsandServer::SetRedisMaxConnections()to configure/update the Redis connection admission limit (default unlimited). - Enforces a “dedicated Redis listener” contract and reserves connection slots in
Acceptorimmediately afteraccept()(beforeSocket::Create()/ TLS). - Adds stats (
ServerStatistics.rejected_redis_connection_count), documentation updates (EN/CN), and unit tests covering rejection behavior and dynamic updates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/brpc_server_unittest.cpp | Adds regression tests for dedicated-listener validation, plaintext rejection, TLS pre-handshake rejection, and dynamic limit updates. |
| src/brpc/server.h | Adds redis_max_connections, rejected_redis_connection_count, and Server::SetRedisMaxConnections() API surface. |
| src/brpc/server.cpp | Validates dedicated Redis-only configuration, wires redis limit into acceptor startup, aggregates rejection stats, implements runtime setter. |
| src/brpc/acceptor.h | Extends StartAccept signature and adds atomics/methods for connection slot accounting and rejection stats. |
| src/brpc/acceptor.cpp | Implements slot reservation before socket creation, relaxed-atomic accounting, and plaintext/TLS rejection behavior. |
| docs/en/server.md | Documents Redis connection limiting, dedicated listener requirements, runtime setter, and stats. |
| docs/cn/server.md | Chinese documentation for the Redis connection limiting feature and behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Problem Summary:
RedisServicehas no server-side connection limit.max_concurrencylimits requests only after protocol parsing, so idle clients can consume all available connection resources. On an SSL listener, accepting excess clients into brpc also allows them to enter the comparatively expensive TLS handshake before an application can reject them.What is changed and the side effects?
Changed:
ServerOptions.redis_max_connections(0keeps the existing unlimited behavior).Server::SetRedisMaxConnections()to atomically change the limit on a running Redis-only Server. Raising or disabling the limit affects subsequent accepts; lowering it does not close established connections.Acceptor, before creating a brpcSocket, so idle clients count and concurrent accepts cannot exceed the limit.redis_serviceset,enabled_protocols="redis", builtin services disabled, and no RPC or other protocol services on thatServer. A dedicated listener can start unlimited and enable the limit later.-ERR max number of clients reachedto excess plaintext clients. If SSL is configured, close the accepted fd immediately without creating a brpcSocketor starting TLS.Serverinstances unlimited, and expose the cumulative rejection count asServerStatistics.rejected_redis_connection_count.Side effects:
ServerOptionsandServerStatistics, so applications using a prebuilt shared brpc library must rebuild with the updated headers and library.Check List:
Tests / Checks:
cmake --build build --target brpc_server_unittest --parallel 4ServerTest.close_idle_connections(4/4 tests).brpc_server_unittestis not clean in this local environment: the existing timing-sensitive overload assertions inServerTest.http_error_codeandServerTest.max_concurrencyobserve a successful third RPC instead of overload rejection. The focused tests above pass; this PR does not change request concurrency code.git diff --check upstream/master...HEADCode / Documentation:
Reviewer focus:
accept, release onSocket::Createfailure orBeforeRecycle, and update the limit with a relaxed store.Rollback:
Call
SetRedisMaxConnections(0)(or start withredis_max_connections=0) to retain unlimited admission, or revert these commits to remove the API and accounting fields.