Skip to content

New RPCConnectionManager: Single unified manager, no ringbuffer, OpenSSL-owned sockets - #8117

Draft
Eddy Ashton (eddyashton) wants to merge 62 commits into
mainfrom
rpc_connection_manager
Draft

New RPCConnectionManager: Single unified manager, no ringbuffer, OpenSSL-owned sockets#8117
Eddy Ashton (eddyashton) wants to merge 62 commits into
mainfrom
rpc_connection_manager

Conversation

@eddyashton

@eddyashton Eddy Ashton (eddyashton) commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

This PR replaces the old split RPC networking path with an OpenSSL-native connection layer. Previously, socket handling lived in host-side libuv code while TLS and protocol sessions were driven through enclave-side ringbuffer messages and memory BIOs. With CCF now running as a single process, that split is no longer useful, so RPC sockets, TLS, protocol session creation, and per-interface policy now live behind a single RPC connection manager.

TLS now terminates at the connection layer. Protocol sessions receive plaintext and write responses through a SessionWriter, so HTTP, HTTP/2, and custom protocols no longer own TLS state directly. This removes the RPC ringbuffer message path, the memory-BIO TLS session layer, and the old libuv RPC connection containers.

The new transport uses non-blocking sockets bound directly to OpenSSL, and splits the work in two. The existing host libuv loop owns the server's own state - accepting connections, uv_poll_t registration, the SSL_CTX, idle connection cleanup via uv_timer_t, and closing file descriptors - and performs no SSL operations itself. Every SSL operation for a connection instead runs on that connection's own OrderedTasks queue, keeping handshakes and bulk encryption off the loop thread. The loop only schedules a pass over a connection, driven either by file descriptor readiness or by a cross-thread request (a queued write, a close, or a certificate update) marshalled through uv_async_t. A connection is serviced by at most one pass at a time. Per-interface behavior such as certificates, session caps, metrics, HTTP parser settings, and custom protocol dispatch is centralized in RPCConnectionManager.

Session caps are applied when a connection is accepted, before any TLS state exists, rather than when its first request arrives. max_open_sessions_hard is documented as a bound on connections, and counting at first-request time missed any client that completed the TCP and TLS handshakes and then went silent while still holding a file descriptor and TLS state.

Node outbound requests are outside this transport and use libcurl.

UDP remains as a small datagram transport driven by uv_poll_t. The temporary QUIC/UDP echo behavior is stateless, so it consumes no session and no interface capacity, while custom UDP protocols are routed to per-peer sessions. Native QUIC is still future work and requires OpenSSL 3.5 or later.

Structural Breakdown

OpenSSLServer is the low-level inbound connection transport. It owns the listening and accepted socket file descriptors, SSL objects, uv_poll_t handles, read/write buffers, handshake state, graceful-close state, certificate reload requests, and idle-timeout sweeping.

OpenSSLSessionManager bridges transport connections to ccf::Session. It lazily creates sessions for inbound connections, forwards plaintext bytes into sessions, implements SessionWriter, and reports connection closure back to the owner.

RPCConnectionManager is the higher-level RPC owner. It replaces the old RPC session container and owns one transport bridge per TCP interface, plus UDP interface state. It applies per-interface admission and caps, certificates, parser settings, application protocol selection, session metrics, custom protocol routing, and UDP peer demultiplexing.

SessionWriter, Session, and PlaintextSession form the new session boundary. Sessions no longer encrypt or decrypt; they parse plaintext and emit plaintext responses to their writer. HTTP/1 and HTTP/2 sessions now use this boundary.

CustomProtocolSubsystemInterface now creates sessions from (ConnID, SessionWriter&) rather than a TLS context. This matches the new layering: custom protocols see plaintext and write through the transport-neutral writer.

DatagramServer is the UDP socket transport. It uses a uv_poll_t handle on the existing libuv loop. RPCConnectionManager echoes datagrams directly for the temporary QUIC behavior, and maps UDP peers to sessions for custom datagram protocols.

Startup wiring moved accordingly. The enclave creates and owns the RPC manager, binds RPC interfaces, resolves actual bound addresses including ephemeral ports, and reports those addresses back through the enclave entry point so the host can write the RPC addresses file.

The removed files are the old RPC transport stack: RPCSessions, TLSSession, host RPC connections, legacy UDP plumbing, the old QUIC session, and the TCP/UDP ringbuffer message types that were specific to the split RPC path. Ledger, consensus, and node-to-node uses of ringbuffer and libuv are not part of this change.

…ransport cert-deferred listening + ALPN + outbound client, RPCConnectionManager (AbstractRPCSessions). Not yet wired into enclave/run.cpp.
…wire RPCConnectionManager into enclave.h/run.cpp, delete RPCSessions/rpc_connections/tls_session. Full build green, 53/53 unit tests pass.
…:Cert::use) and request client cert on inbound for caller auth; add peer-cert capture test. Full build green, unit tests pass.
…xing localhost/[::1] interfaces (cpp, cpp_cose_only, common_ipv6 e2e). Add localhost/IPv6 binding tests.
… large response queued just before close_socket() is not truncated (fixes cpp/cpp_cose_only receipt 'server disconnected'). Add truncation test.
…e (ERR_clear_error) before each SSL op so a stale error from one connection cannot poison SSL_get_error for another (root cause of cpp/cpp_cose_only 'server disconnected'). Add persistent-connection + peer-cert tests.
… handler; branch udp interfaces to listen_udp. Clearly marked QUIC extension points (substrate for OpenSSL >=3.5 native QUIC). e2e_logging udp echo passes; full suite green.
…ned quic_session.h/src/quic, udp.h + udp/msg_types.h + UDPImpl vestiges in run.cpp, dead RPC ringbuffer message enums (keep tcp::ConnID). Drop old-implementation comments. Fix build after cert.h use->configure_ssl rename + commit-callback include.
…Config (so the enclave-side RPC manager receives it); track per-connection last_active in OpenSSLServer and sweep idle connections off the epoll timeout. Plumbed manager->bridge->server. idletimeout e2e passes.

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 replaces the legacy split host/enclave RPC networking stack (ringbuffer-driven sockets + memory-BIO TLS) with a single-process, OpenSSL-native connection layer, and updates the session boundary so HTTP/HTTP2/custom protocols operate on plaintext via a transport-provided SessionWriter.

Changes:

  • Introduces RPCConnectionManager + OpenSSLSessionManager + OpenSSLServer-backed transports, removing the old ringbuffer RPC message types, TLSSession, and host libuv RPC containers.
  • Refactors protocol sessions (HTTP/1 + HTTP/2) to run as PlaintextSession instances which write responses through SessionWriter rather than owning TLS state.
  • Adds a minimal epoll-based UDP DatagramServer with a temporary echo session for QUIC/UDP behavior, and updates docs/tests accordingly.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 56 out of 57 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/tls_groups.py Updates test comment to reference new OpenSSL-native TLS group source header.
tests/infra/clients.py Makes client context managers exception-safe via try/finally close.
tests/e2e_operations.py Fixes ledger chunk skipping logic when chunk end seqno is None (open chunk).
tests/connections.py Makes socket context manager exception-safe via try/finally close.
src/udp/msg_types.h Removes UDP ringbuffer message types from old split RPC transport.
src/tls/tls.h Removes legacy TLS error-code shim header tied to the old BIO/ringbuffer model.
src/tls/test/main.cpp Removes legacy TLS unit tests for the old in-enclave TLS path.
src/tls/server.h Removes legacy TLS server wrapper around ccf::tls::Context.
src/tls/README.md Removes docs describing the deprecated TLS emulation layer.
src/tls/plaintext_server.h Removes legacy plaintext server adapter that mimicked TLS Context.
src/tls/context.h Removes legacy TLS Context implementation (memory BIO + negated error codes).
src/tls/client.h Removes legacy TLS client wrapper around ccf::tls::Context.
src/tcp/msg_types.h Drops TCP ringbuffer message definitions, keeping only ConnID.
src/quic/test/main.cpp Removes legacy QUIC OpenSSL call test tied to removed TLS headers.
src/quic/quic_session.h Removes old QUIC session implementation built on ringbuffer UDP messages.
src/node/rpc/custom_protocol_subsystem.h Updates custom protocol session creation to use (ConnID, SessionWriter&).
src/node/node_state.h Switches node RPC session ownership to AbstractRPCSessions.
src/node/http_node_client.h Adds missing include needed by refactored HTTP node client code.
src/js/extensions/ccf/crypto.cpp Replaces TLS CA-based X509 bundle check with direct OpenSSL parsing helpers.
src/http/test/curl_test.cpp Updates transient curl error classification tests (adds CURLE_SSL_CONNECT_ERROR).
src/http/http2_session.h Migrates HTTP/2 server session to PlaintextSession + explicit peer cert passing.
src/http/http_session.h Migrates HTTP/1 server session to PlaintextSession + explicit peer cert passing.
src/http/http_proc.h Removes dependency on old tls_session.h by switching includes.
src/http/http_parser.h Removes dependency on old tls_session.h by switching includes.
src/http/curl.h Treats CURLE_SSL_CONNECT_ERROR as retryable and documents rationale.
src/host/udp.h Removes legacy libuv UDP implementation used by the old split transport.
src/host/tls/openssl_session_manager.h Adds transport-to-session bridge implementing SessionWriter over OpenSSL-native sockets.
src/host/test/rpc_connections.cpp Removes tests for legacy host RPC connection containers.
src/host/run.cpp Removes old host-side RPC interface setup; host now writes rpc addresses returned by enclave.
src/host/rpc_connections.h Removes legacy host RPC connection container.
src/host/rpc_connection_manager.h Adds new unified RPC connection manager (per-interface policy + transports + UDP demux).
src/host/datagram_server.h Adds epoll-based UDP server substrate (future QUIC extension point).
src/host/datagram_echo_session.h Adds temporary UDP echo session used for current QUIC/UDP behavior.
src/host/configuration.h Removes host config idle timeout field (now part of startup config / RPC manager).
src/enclave/tls_session.h Removes legacy enclave TLS session (ringbuffer/BIO-based).
src/enclave/session.h Refactors session boundary to plaintext + SessionWriter; updates handle_incoming_data signature.
src/enclave/session_writer.h Adds SessionWriter abstraction for transport-owned output.
src/enclave/rpc_sessions.h Removes legacy in-enclave RPCSessions container.
src/enclave/no_more_sessions.h Adds NoMoreSessionsImpl for soft session cap behavior with immediate plaintext 503.
src/enclave/main.cpp Extends enclave entry to return resolved RPC addresses to host.
src/enclave/entry_points.h Extends entry point signature to return resolved RPC addresses to host.
src/enclave/enclave.h Wires new RPCConnectionManager, binds interfaces in enclave, and stops transports on shutdown.
src/enclave/abstract_rpc_sessions.h Introduces transport-agnostic RPC sessions interface used by node/frontends.
src/common/configuration.h Adds idle_connection_timeout to JSON (startup config) serialization.
src/clients/tls/test/main.cpp Adds new unit tests for client tooling TLS certificate helpers.
src/clients/tls/README.md Documents that TLS helpers are client-tooling-only (node uses host/tls + curl).
src/clients/tls/cert.h Updates includes to new client-tooling TLS CA/cert helpers.
src/clients/tls/ca.h Adds new client-tooling CA helper (OpenSSL store configuration).
src/clients/tls_client.h Updates includes to new client-tooling TLS CA/cert helpers.
include/ccf/research/custom_protocol_subsystem_interface.h Updates custom protocol interface to accept SessionWriter& instead of TLS context.
include/ccf/node/startup_config.h Adds idle_connection_timeout to startup config API.
include/ccf/node/session.h Extends session interface to accept source sockaddr for datagram transports.
doc/contribute/onboarding.rst Updates onboarding diagram to reflect TLS termination in connection layer + writer boundary.
doc/architecture/tls_internals.rst Rewrites TLS internals doc to match new OpenSSL-native connection design.
CMakeLists.txt Replaces removed TLS/rpc connection tests with openssl_server_test and new client TLS helper tests.
Suppressed comments (1)

src/host/rpc_connection_manager.h:547

  • port can be empty for port-less/ephemeral bind addresses. std::stoi(port) will throw in that case, so UDP interfaces configured with an unspecified port cannot start. Parse empty as 0 (ephemeral) before converting.
      udp->server = std::make_unique<asynchost::DatagramServer>(
        host,
        static_cast<uint16_t>(std::stoi(port)),
        [this, li, udp_ptr, writer](

Comment thread src/host/rpc_connection_manager.h Outdated
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Description

Comparing 5 available runs from this branch (#8117) against the trend of the last 30 main runs.

Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent main runs, using a 7-run half-life. The 5 orange branch lines run from the oldest (faintest) to the latest (darkest and thickest); the darker blue band is the main baseline +/- 1 std dev and the lighter blue band around it is +/- 2 std dev.

Axis labels show the latest branch value and its difference from the main EWMA baseline, where 0% is on the baseline. They are coloured green where the latest run improves on the baseline, red where it regresses, and grey where the difference is within one std dev of the baseline (within noise). Higher is better for throughput and rate, lower for latency and memory.

Throughput (tx/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#E5484D!important}
    .radarAxisLabel:nth-of-type(2){fill:#E5484D!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#E5484D!important}
    .radarAxisLabel:nth-of-type(5){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#E5484D!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic: 57,828 tx/s ▼ 11%"]
  axis b1["Basic Blocking: 707 tx/s ▼ 30%"]
  axis b2["Basic JS: 4,925 tx/s ▬ +6%"]
  axis b3["Basic Multi-Threaded: 57,896 tx/s ▼ 31%"]
  axis b4["Historical Queries: 244,712 tx/s ▲ 21%"]
  axis b5["Logging: 56,880 tx/s ▬ -6%"]
  axis b6["Logging JWT: 8,712 tx/s ▼ 12%"]
  curve stddev2_high["main EWMA + 2 std dev"]{112.03, 104.69, 111.34, 113.38, 122.06, 111.99, 111.84}
  curve stddev1_high["main EWMA + 1 std dev"]{106.01, 102.35, 105.67, 106.69, 111.03, 106.00, 105.92}
  curve stddev1_low["main EWMA - 1 std dev"]{93.99, 97.65, 94.33, 93.31, 88.97, 94.00, 94.08}
  curve stddev2_low["main EWMA - 2 std dev"]{87.97, 95.31, 88.66, 86.62, 77.94, 88.01, 88.16}
  curve branch_0["#8117 (4 runs earlier)"]{111.75, 72.17, 106.23, 126.78, 70.05, 110.54, 107.26}
  curve branch_1["#8117 (3 runs earlier)"]{111.57, 78.82, 108.05, 127.96, 96.23, 108.31, 106.85}
  curve branch_2["#8117 (2 runs earlier)"]{108.70, 72.72, 108.75, 85.07, 71.55, 111.68, 105.96}
  curve branch_3["#8117 (1 run earlier)"]{90.64, 74.01, 106.41, 70.88, 119.26, 95.77, 108.60}
  curve branch_4["#8117"]{88.88, 70.33, 105.59, 69.47, 121.47, 94.29, 87.85}
  graticule polygon
  max 149
  min 48
  ticks 0
  showLegend false
Loading

Latency (ms)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(3){fill:#E5484D!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Commit Latency 16ms: 4.64 ms ▬ -27%"]
  axis b1["Commit Latency 1ms: 1.7 ms ▼ 6%"]
  axis b2["Commit Latency 256ms: 210 ms ▲ 1%"]
  curve stddev2_high["main EWMA + 2 std dev"]{163.07, 106.11, 101.47}
  curve stddev1_high["main EWMA + 1 std dev"]{131.53, 103.06, 100.73}
  curve stddev1_low["main EWMA - 1 std dev"]{68.47, 96.94, 99.27}
  curve stddev2_low["main EWMA - 2 std dev"]{36.93, 93.89, 98.53}
  curve branch_0["#8117 (4 runs earlier)"]{93.30, 61.87, 101.70}
  curve branch_1["#8117 (3 runs earlier)"]{82.02, 64.78, 101.92}
  curve branch_2["#8117 (2 runs earlier)"]{84.56, 95.66, 102.43}
  curve branch_3["#8117 (1 run earlier)"]{100.36, 95.61, 100.43}
  curve branch_4["#8117"]{73.15, 94.10, 101.02}
  graticule polygon
  max 208
  ticks 0
  showLegend false
Loading

Memory (bytes)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(3){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(4){fill:#E5484D!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#2DA44E!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic: 88.2 MiB ▬ +2%"]
  axis b1["Basic Blocking: 71 MiB ▼ 1%"]
  axis b2["Basic JS: 67.7 MiB ▼ 5%"]
  axis b3["Basic Multi-Threaded: 91.7 MiB ▲ 3%"]
  axis b4["Logging: 75.3 MiB ▬ 0%"]
  axis b5["Logging JWT: 66.1 MiB ▼ 4%"]
  curve stddev2_high["main EWMA + 2 std dev"]{106.22, 100.66, 103.82, 102.09, 102.05, 101.54}
  curve stddev1_high["main EWMA + 1 std dev"]{103.11, 100.33, 101.91, 101.05, 101.02, 100.77}
  curve stddev1_low["main EWMA - 1 std dev"]{96.89, 99.67, 98.09, 98.95, 98.98, 99.23}
  curve stddev2_low["main EWMA - 2 std dev"]{93.78, 99.34, 96.18, 97.91, 97.95, 98.46}
  curve branch_0["#8117 (4 runs earlier)"]{102.92, 101.16, 97.53, 106.34, 100.72, 96.76}
  curve branch_1["#8117 (3 runs earlier)"]{102.80, 101.11, 95.72, 106.92, 99.53, 96.50}
  curve branch_2["#8117 (2 runs earlier)"]{99.69, 99.40, 94.15, 101.36, 98.54, 95.62}
  curve branch_3["#8117 (1 run earlier)"]{98.93, 99.29, 94.66, 101.94, 99.52, 95.89}
  curve branch_4["#8117"]{102.31, 99.35, 94.97, 102.52, 99.51, 96.16}
  graticule polygon
  max 112
  min 89
  ticks 0
  showLegend false
Loading

Rate (ops/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#2DA44E!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["CHAMP get: 38,338,419 ops/s ▬ +4%"]
  axis b1["CHAMP put: 5,558,496 ops/s ▬ +5%"]
  axis b2["KV deserialisation: 1,615,770 ops/s ▬ +2%"]
  axis b3["KV serialisation: 1,474,926 ops/s ▬ +4%"]
  axis b4["KV snapshot deserialis...: 4,180 ops/s ▬ +3%"]
  axis b5["KV snapshot serialisation: 4,739 ops/s ▲ 8%"]
  curve stddev2_high["main EWMA + 2 std dev"]{110.07, 111.17, 109.74, 109.66, 110.06, 113.87}
  curve stddev1_high["main EWMA + 1 std dev"]{105.04, 105.58, 104.87, 104.83, 105.03, 106.93}
  curve stddev1_low["main EWMA - 1 std dev"]{94.96, 94.42, 95.13, 95.17, 94.97, 93.07}
  curve stddev2_low["main EWMA - 2 std dev"]{89.93, 88.83, 90.26, 90.34, 89.94, 86.13}
  curve branch_0["#8117 (4 runs earlier)"]{102.60, 103.20, 104.56, 101.59, 102.15, 101.77}
  curve branch_1["#8117 (3 runs earlier)"]{102.87, 103.29, 103.37, 102.20, 102.72, 114.47}
  curve branch_2["#8117 (2 runs earlier)"]{103.00, 103.03, 101.53, 101.46, 101.93, 106.19}
  curve branch_3["#8117 (1 run earlier)"]{102.41, 102.97, 102.20, 103.23, 100.19, 97.10}
  curve branch_4["#8117"]{103.68, 104.66, 102.20, 103.85, 102.60, 107.77}
  graticule polygon
  max 125
  min 76
  ticks 0
  showLegend false
Loading

@eddyashton
Eddy Ashton (eddyashton) marked this pull request as draft August 5, 2026 12:18
@achamayou

Amaury Chamayou (achamayou) commented Aug 6, 2026

Copy link
Copy Markdown
Member

Had an agent look at why performance seems affected, particularly on the blocking commit benchmark. I found the Nagle regression right away, which seemed worth fixing. The rest is more debatable, some of the locking changes sound like easy wins, but need testing, but the big one is probably not having handshake handling delaying permanent regime requests for other sessions, and that requires non trivial re-design I think.

Executive summary

PR 8117 consistently regresses the Basic Blocking benchmark by roughly 26-28% against the recent main EWMA. The four observed branch results are 725.5, 792.4, 731.1, and 744.0 tx/s. The latest result is 744.0 tx/s, 26% below a main EWMA of approximately 1,010 tx/s.

The missing TCP_NODELAY setting was a real regression from the old TCP transport and has been restored in commit e2d94caa3. The old transport called uv_tcp_nodelay(..., 1) for every socket, while the new accept4() path initially left Nagle enabled. Runtime tracing confirms that accepted sockets now receive setsockopt(..., TCP_NODELAY, 1). However, the post-fix CI result improved only from 731.1 to 744.0 tx/s and remains 26% below main. Nagle was therefore not the principal cause.

The most likely remaining cause is the new architecture's concentration of TLS and socket work on one libuv loop, amplified by the benchmark's short, highly concurrent, latency-sensitive shape. PR 8117 moves SSL_read() and SSL_write() from per-session worker processing to the shared default libuv loop. Every response also crosses a mutexed queue and an async wake before that loop performs TLS encryption. With 128 clients each allowing exactly one request in flight, added response delay translates directly into lower throughput; pipelined Basic can hide the same latency.

Benchmark shape

pi_basic_blocking is configured in CMakeLists.txt as:

128 clients
100 blocking writes per client
primary target
max-writes-ahead = 0

The endpoint waits for consensus commitment before responding. Each client sends its next request only after receiving the previous response. Aggregate throughput is therefore approximately:

active clients / average request-response latency

At the main baseline, 128 / 1,010 is about 127 ms per request. At 744 tx/s it is about 172 ms per request, an added delay of roughly 45 ms in the fully-overlapped idealisation.

This benchmark also launches 128 client processes sequentially and exports throughput over the interval from the earliest send to the latest receive. Since each client sends only 100 requests, connection establishment and launch skew are a significant part of the measured interval. A slower 128-way TLS handshake ramp can reduce the reported throughput even if steady-state request handling is unchanged.

Evidence

CI A/B results

Run Basic Blocking throughput Relative to current main EWMA
PR run 1 725.5 tx/s -28%
PR run 2 792.4 tx/s -22%
PR run 3 731.1 tx/s -28%
After TCP_NODELAY 744.0 tx/s -26%

The regression is present in every PR run and survives the TCP_NODELAY fix. Other benchmarks do not move consistently with it: pipelined Basic was above baseline in the first three runs, while commit-latency microbenchmarks remained close to baseline. This points away from KV execution and consensus as the primary source.

Local measurements

The local 128-client benchmark is noisy because process startup is serialized and this development host is shared:

Variant Full-run throughput Client start spread All-client overlap All-active throughput
TCP_NODELAY, run 1 471 tx/s about 16.5 s 3.7 s 1,039 tx/s
Nagle enabled 504 tx/s 14.9 s none invalid
TCP_NODELAY, run 2 622 tx/s 9.9 s 10.3 s 1,180 tx/s

The fixed full-run results vary too much to claim a local throughput win. The all-active fixed rates are nevertheless near or above the main CI baseline, while much of the full-run interval is launch ramp rather than steady load.

A stable 16-client variant with 1,000 requests per client produced 99% overlap and nearly identical results:

Variant Throughput p50 latency p90 latency p99 latency
TCP_NODELAY 159.50 tx/s 100.124 ms 102.784 ms 105.760 ms
Nagle enabled 159.34 tx/s 100.109 ms 102.830 ms 106.397 ms

At 16 clients the 100 ms signature cadence dominates, so this test does not exercise the high-concurrency bottleneck. It does show that Nagle is not a general per-request 25% cost.

CPU profiling was not available in the dev container: perf is not installed and /proc/sys/kernel/perf_event_paranoid is 4.

Hot-path comparison

Old path

The legacy design performed TLS work in each TLSSession, reached from the session's ordered worker task. Encrypted bytes then crossed the ringbuffer and were written by libuv. TCP sockets were configured with both TCP_NODELAY and keepalive.

New path

The new response path is:

HTTP response vector
  -> ThreadedSession::SendDataTask
  -> PlaintextSession::send_data_thread
  -> SessionWriter::write_outbound(span)
  -> OpenSSLServer::send
  -> pending_out under out_mutex
  -> uv_async_send
  -> one libuv loop drains pending_out
  -> copy into Conn::outbuf
  -> SSL_write on the loop thread
  -> socket send

All inbound handshakes, SSL_read() calls, SSL_write() calls, connection-map operations, and poll-interest updates for all RPC interfaces run on uv_default_loop(). This creates a single serialization point that did not exist when TLS processing happened in ordered worker tasks.

Likely contributors

1. Single-loop TLS encryption and handshake serialization

Confidence: high as an architectural bottleneck; medium as the full explanation for the measured 26%.

OpenSSLServer performs every handshake and TLS record operation on the shared libuv loop. Basic Blocking creates 128 TLS connections and waits synchronously for every small response. A burst of committed callbacks queues many responses, but one loop encrypts and writes them serially. Initial handshakes are also serialized on this loop, potentially widening the benchmark's client start ramp.

Recommended measurement:

  • Run the benchmark with CPU sampling enabled on the CI benchmark host.
  • Separate samples by the basic server process and inspect SSL_write, SSL_read, SSL_accept, drain_pending_out, uv__io_poll, and task-system frames.
  • Record handshake completion timestamps and first-request timestamps for all 128 connections.
  • Compare full-run throughput with all-clients-active throughput.

Potential improvement:

  • Give RPC transport work dedicated loop threads, or shard connections across several loops.
  • At minimum, separate expensive handshake processing from steady-state response encryption if OpenSSL object ownership can remain safe.
  • This is a larger design change and should follow profiling rather than be attempted speculatively.

2. Lifecycle mutex on every socket event and response wake

Confidence: medium.

on_connection_poll() calls mark_loop_thread(), which locks lifecycle_mutex on every poll callback and rewrites the same thread ID. send() calls wake(), which takes the same mutex for every response. Under 128 active connections, the loop and worker threads repeatedly contend on a mutex whose steady-state information rarely changes.

Potential improvement:

  • Set the loop thread ID once, rather than on every callback.
  • Make the steady-state stop/initialisation flags atomic or otherwise arrange lock-free reads in wake().
  • Keep the mutex for startup/shutdown transitions only.

This is relatively contained and should be measurable with mutex-contention profiling or a counter around failed/immediate lock acquisition.

3. Two avoidable response copies

Confidence: high that the copies exist; low-to-medium impact for tiny Basic responses.

ThreadedSession already owns the response as std::vector<uint8_t>, but SessionWriter::write_outbound accepts only a span. OpenSSLServer::send copies that span into a new OutItem::data, and drain_pending_out copies it again into Conn::outbuf before SSL_write.

Potential improvement:

  • Add an ownership-taking writer API such as write_outbound(ConnID, std::vector<uint8_t>&&).
  • Move the vector into OutItem.
  • When Conn::outbuf is empty, move/swap OutItem::data into it rather than inserting.
  • Preserve the span overload for callers that cannot transfer ownership.

This should help larger responses and high-throughput workloads even if it is not the main Basic Blocking regression.

4. Unconditional wake and queue synchronization

Confidence: medium-low, because libuv already coalesces async notifications.

Every send() locks out_mutex, appends to pending_out, then calls uv_async_send. Libuv coalesces pending async callbacks, but the application still executes the wake path and lifecycle lock for every response.

Potential improvement:

  • Wake only when the queue transitions from empty to non-empty.
  • Drain until no items remain, with a carefully designed pending flag to avoid missed wakeups.
  • Reserve or use a queue structure that avoids repeated vector growth under bursts.

5. Repeated poll re-arming

Confidence: medium-low until syscall counts are collected.

finish_or_close() calls update_interest() after every successful response, and update_interest() calls uv_poll_start() even when the desired mask remains UV_READABLE. Cache the current event mask in Conn and re-arm only when UV_WRITABLE interest actually changes. Validate the benefit by counting epoll_ctl/poll-update syscalls before and after.

6. Session-map mutex on every inbound chunk

Confidence: low-to-medium.

OpenSSLSessionManager::on_data() takes sessions_mutex for every decrypted chunk, including every request on an established persistent connection. The loop is the sole inbound caller, while close operations may arrive from workers.

Potential improvement:

  • Associate the session directly with connection state after first creation, or separate loop-owned lookup from cross-thread close coordination.
  • Avoid changing this without race-focused tests; the current lock is simple and correct.

Benchmark improvements

The current benchmark metric conflates connection startup and steady-state request throughput. Both are useful, but they should be reported separately.

Recommended changes:

  1. Add a start barrier so all submitters connect and wait before requests begin.
  2. Export all_clients_active_average_throughput_tx/s to Bencher alongside the existing full-run throughput.
  3. Increase requests per client so the steady window dominates client process and TLS setup.
  4. Add explicit connection-ramp metrics: time from first to last handshake and first request.
  5. Retain the existing metric under a name such as Basic Blocking including connection ramp if startup performance is intentional.

These changes would show whether PR 8117 regresses TLS connection establishment, steady blocking response latency, or both.

Recommended next actions

  1. Keep TCP_NODELAY commit e2d94caa3; it restores an explicit legacy socket policy even though it does not recover the benchmark.
  2. Re-run or extend CI with handshake timestamps and all-active throughput.
  3. Profile the server process on the benchmark runner, focusing on the shared libuv TLS loop and lifecycle mutex.
  4. Prototype the low-risk mutex cleanup and ownership-taking response path independently, benchmarking each change.
  5. Consider loop sharding only if profiling confirms SSL_accept/SSL_write serialization dominates.

Validation performed

  • openssl_server_test: 17 test cases passed, 368 assertions passed, 1 skipped.
  • C++ format checks passed for src/host/tls/openssl_server.h.
  • strace captured successful setsockopt(..., TCP_NODELAY, 1) calls on accepted sockets.
  • Post-fix CI benchmark completed successfully at 744.0 Basic Blocking tx/s.
  • Multiple local Basic Blocking runs were collected as described above.

The TCP_NODELAY fix was committed and pushed as e2d94caa3. This report is intentionally not committed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bench-ab run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants