Skip to content

feat: limit Redis server connections - #3498

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

feat: limit Redis server connections#3498
thweetkomputer wants to merge 2 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
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.

1 participant