Skip to content

feat: limit Redis connections and fix GCC 13 test build - #29

Merged
thweetkomputer merged 3 commits into
masterfrom
fix/baidu-thread-local-cstdint
Aug 28, 2026
Merged

feat: limit Redis connections and fix GCC 13 test build#29
thweetkomputer merged 3 commits into
masterfrom
fix/baidu-thread-local-cstdint

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

  • RedisService has no server-side connection limit. Request concurrency limits run after protocol parsing, so idle clients can exhaust connection resources, and excess TLS clients can enter an expensive handshake before the application rejects them.
  • test/baidu_thread_local_unittest.cpp uses uintptr_t without directly including <cstdint>, which fails to compile with GCC 13.

What is changed and the side effects?

Changed:

  • Add <cstdint> to the thread-local unit test.
  • Add ServerOptions.redis_max_connections; 0 preserves unlimited admission.
  • Add thread-safe Server::SetRedisMaxConnections(size_t) so a running Redis-only listener can atomically enable, disable, raise, or lower the limit.
  • Reserve connection slots atomically immediately after accept, before creating a brpc Socket, so idle clients count and concurrent accepts cannot exceed the limit.
  • Require a non-zero 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.
  • Return -ERR max number of clients reached to excess plaintext clients. When SSL is configured, close the accepted fd before creating a brpc Socket or starting TLS authentication.
  • Keep internal acceptors and other Server instances unaffected, and expose cumulative rejections through ServerStatistics.rejected_redis_connection_count.
  • Lowering the runtime limit keeps existing connections and rejects new ones until the count drops below the new value; raising it applies to subsequent admission checks.
  • Add English/Chinese documentation and regression tests for configuration isolation, plaintext rejection, independent RPC availability, runtime limit changes, slot recovery, and pre-TLS rejection.

Side effects:

  • Performance effects(性能影响): Each accepted connection performs relaxed atomic connection accounting and loads the current atomic limit; limited Redis listeners use a relaxed CAS for admission. Runtime updates are one relaxed atomic store. There is no request hot-path cost.

  • Breaking backward compatibility(向后兼容性): Default runtime behavior remains unlimited. Adding fields changes the in-memory layouts of ServerOptions and ServerStatistics, so consumers must rebuild their application and brpc library together.


Check List:

  • Rebasing the PR branch onto current eloqdata/brpc master leaves only the two requested changes and their tests/docs.
  • GCC 13 builds test_butil and brpc_server_unittest with --parallel 4.
  • BaiduThreadLocalTest.* passes (4 tests).
  • The Redis connection-limit tests pass, including enabling after startup, raising and lowering the limit, preserving existing connections, and rejecting the setter on an RPC Server.
  • ServerTest.close_idle_connections passes.
  • IO_URING_ENABLED=ON builds brpc_server_unittest with --parallel 4.
  • io_uring runtime tests were not completed in this container: brpc aborts during startup because the kernel rejects buffer-ring registration, before the tested acceptor code runs.
  • The complete unit-test suite was not run; targeted tests cover the changed behavior.
  • git diff --check origin/master...HEAD passes.
  • Followed the Contributor Covenant Code of Conduct.

Reviewer focus:

  • The dedicated-listener requirement: protocol identification occurs after TLS, so this invariant allows excess TLS connections to be dropped before authentication without limiting other RPC interfaces.
  • The connection-slot lifecycle: reserve after accept, release on Socket::Create failure or BeforeRecycle, and update/read the runtime limit with relaxed ordering because the atomics publish no socket state.
  • EloqData's io_uring accept path remains intact; admission occurs before selecting the socket event handler.

Rollback:

Set redis_max_connections to 0 to retain unlimited admission, or revert the Redis-limit commit. The <cstdint> change is independent and test-only.

@thweetkomputer
thweetkomputer force-pushed the fix/baidu-thread-local-cstdint branch from eccbe13 to c8de3fb Compare August 28, 2026 04:35
@thweetkomputer thweetkomputer changed the title fix: include cstdint in thread-local test feat: limit Redis connections and fix GCC 13 test build Aug 28, 2026
@thweetkomputer
thweetkomputer marked this pull request as ready for review August 28, 2026 04:40

@zhangh43 zhangh43 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.

+1

@thweetkomputer
thweetkomputer merged commit 34e0e01 into master Aug 28, 2026
9 checks passed
@thweetkomputer
thweetkomputer deleted the fix/baidu-thread-local-cstdint branch August 28, 2026 08:01
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