Skip to content

feat: limit Redis server connections - #3498

Open
thweetkomputer wants to merge 3 commits into
apache:masterfrom
thweetkomputer:feature/redis-max-connections
Open

feat: limit Redis server connections#3498
thweetkomputer wants to merge 3 commits into
apache:masterfrom
thweetkomputer:feature/redis-max-connections

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

RedisService has no server-side connection limit. max_concurrency limits 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:

  • Add ServerOptions.redis_max_connections (0 keeps the existing unlimited behavior).
  • Add 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.
  • Reserve connection slots atomically in Acceptor, before creating a brpc Socket, so idle clients count and concurrent accepts cannot exceed the limit.
  • Require the limit to use a dedicated Redis public listener: redis_service set, enabled_protocols="redis", builtin services disabled, and no RPC or other protocol services on that Server. A dedicated listener can start unlimited and enable the limit later.
  • Return -ERR max number of clients reached to excess plaintext clients. If SSL is configured, close the accepted fd immediately without creating a brpc Socket or starting TLS.
  • Keep internal acceptors and other Server instances unlimited, and expose the cumulative rejection count as ServerStatistics.rejected_redis_connection_count.
  • Document the startup option and runtime setter in the English and Chinese server guides and add regression tests for listener isolation, dynamic limit changes, plaintext rejection, independent RPC availability, slot recovery, and pre-TLS rejection.

Side effects:

  • Performance effects: accepted connections perform relaxed atomic admission/accounting, including an atomic load of the current limit; runtime updates are a relaxed atomic store. There is no request hot-path cost.
  • Breaking backward compatibility: the default behavior remains unlimited and existing source configuration is unchanged. The new public fields change the in-memory layouts of ServerOptions and ServerStatistics, 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 4
  • Focused tests pass: dedicated-listener validation, runtime enable/raise/lower behavior, RPC isolation, plaintext rejection, pre-TLS rejection, and ServerTest.close_idle_connections (4/4 tests).
  • The complete brpc_server_unittest is not clean in this local environment: the existing timing-sensitive overload assertions in ServerTest.http_error_code and ServerTest.max_concurrency observe 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...HEAD

Code / Documentation:

  • The code follows the repository style.
  • Public API behavior and the accept/TLS invariants are documented.
  • The final diff was reviewed against the merge base.

Reviewer focus:

  • Whether requiring a dedicated Redis listener is the desired public contract. Protocol identification happens after TLS, so enforcing this invariant is what lets the acceptor reject excess TLS connections before authentication without affecting RPC listeners.
  • The relaxed atomic slot lifecycle: load the mutable limit and reserve immediately after accept, release on Socket::Create failure or BeforeRecycle, and update the limit with a relaxed store.

Rollback:

Call SetRedisMaxConnections(0) (or start with redis_max_connections=0) to retain unlimited admission, or revert these commits to remove the API and accounting fields.

@thweetkomputer
thweetkomputer marked this pull request as ready for review August 28, 2026 04:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_connections and Server::SetRedisMaxConnections() to configure/update the Redis connection admission limit (default unlimited).
  • Enforces a “dedicated Redis listener” contract and reserves connection slots in Acceptor immediately after accept() (before Socket::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.

Comment thread src/brpc/acceptor.cpp Outdated
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.

2 participants