Skip to content

[rpc] Isolate internal listener resources from client backpressure - #4857

Open
platinumhamburg wants to merge 3 commits into
apache:mainfrom
platinumhamburg:rpc-listener-isolation
Open

platinumhamburg wants to merge 3 commits into
apache:mainfrom
platinumhamburg:rpc-listener-isolation

Conversation

@platinumhamburg

Copy link
Copy Markdown
Contributor

Purpose

Closes #4856.

Client requests can occupy a request processor and fill its queue, causing backpressure to pause associated replication connections. Give a separately configured internal listener its own RPC resources so client overload does not directly stop replication and control traffic.

Brief change log

  • Select shared or isolated resources automatically from the existing listener configuration. A single endpoint retains one resource group; a separate internal listener gets an additional group, while all external listeners share the external group.
  • Reuse the same network/resource implementation, request processors, queue backpressure, and protocol handlers. Each group uses the existing network-thread, worker-thread, and queue-capacity settings.
  • Keep existing request metrics shared and retain their names, scopes, labels, classifications, and timing boundaries. Preserve external processor indices, append internal indices, and report the total queue size across both groups.
  • Close both resource groups and clean up partially started resources if binding a listener fails.

Tests

  • Reproduced the regression before implementation: an internal RPC timed out while an external request blocked the shared worker.
  • Java 11 Maven verify for the RPC module and its build dependencies, selecting all RPC test classes: 165 tests, 163 passed and 2 existing skips; no failures or errors. Checkstyle, Spotless, and license checks passed.
  • Nine cases in NettyServerListenerIsolationTest cover replication under external backpressure, independent network processing, single-listener compatibility, shared external capacity, metric compatibility, shutdown, and failed-bind cleanup.

API and Format

No changes to RPC interfaces, wire or storage formats, authentication, authorization, or request ordering. No isolation enable switch or additional configuration options. Separate internal and external listeners allocate two groups of the configured capacity; single-listener deployments retain one group.

Documentation

Document listener resource isolation and capacity allocation in the security overview.

@platinumhamburg
platinumhamburg marked this pull request as ready for review October 24, 2026 13:56
@Jackeyzhe

Copy link
Copy Markdown
Contributor

This PR currently conflicts with main, which now closes request handlers in RequestProcessorPool.closeAsync(). Since the two resource groups in this PR share the same handler instances, could the rebase ensure they are closed only once, after both worker pools have stopped? A shutdown test covering handler closure would help guard that lifecycle.

@platinumhamburg

Copy link
Copy Markdown
Contributor Author

Hi @Jackeyzhe , Thanks for pointing this out. Rebased onto the latest main. Shared request handlers are now owned by NettyServer and closed once, after both listener resource groups finish shutting down. Repeated closeAsync() calls return the same termination future.

Added NettyServerShutdownTest, which blocks a request in each resource group and releases them separately to verify that the shared handler remains open until both finish, then closes exactly once. The shutdown and listener-isolation tests pass in CI.

@polyzos

polyzos commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Thanks @platinumhamburg, the change looks good. Before merging:

netty.server.num-network-threads, num-worker-threads and max-queued-requests now apply per resource group, but their descriptions still describe one server-wide pool. Should we update them in ConfigOptions and configuration.md, and note that a separate internal listener doubles them??. The Helm chart already uses separate INTERNAL/CLIENT listeners, so every Helm deployment gets this on upgrade.

Or this can be a follow-up PR, so whatever you prefer 🙏 I approved it, as it LGTM

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

Thanks for the update. The shared handler is now closed once after both resource groups finish, and the new test covers the shutdown ordering and repeated close calls. I don't see any remaining blocking issues. LGTM.

@platinumhamburg

Copy link
Copy Markdown
Contributor Author

Thanks @polyzos, agreed. Updated all three descriptions in ConfigOptions and configuration.md to clarify that they apply per resource group, and documented the upgrade impact for the default Helm INTERNAL/CLIENT setup: two groups, with 6 network threads and 16 worker threads in total per server using the defaults. Also clarified that max-queued-requests controls queue backpressure rather than a hard queue limit. Single-listener deployments still use one shared group.

Addressed in 325da82. Java 11 compilation, configuration documentation generation, Spotless, Checkstyle, and license checks passed.

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.

[rpc] Isolate internal listener resources from client backpressure

3 participants