Skip to content

[ISSUE #8194] Add opt-in named upstream connection-pool groups for workload isolation - #8299

Merged
Aias00 merged 1 commit into
apache:masterfrom
konlue:feat/8194-pool-groups
Nov 10, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
konlue:feat/8194-pool-groups

Conversation

@konlue

@konlue konlue commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #8194.

Problem

HttpClientFactory builds a single ConnectionProvider from process-wide configuration and both built-in HTTP plugins share one client, so a slow bulk workload calling the same upstream origin can starve an interactive one through that shared pool. There is no way to give explicitly configured workloads their own bounded connection pool, and per-origin pools do not help because the contention is between workloads, not origins.

Change

This PR adds opt-in named upstream pool groups defined only in trusted startup configuration. shenyu.httpclient.pool-groups supports at most 16 groups; every group must use the FIXED type, have an explicit unique name other than shenyu-proxy, a positive maxConnections, and an omitted-or-positive maxPendingAcquires (DISABLED, ELASTIC, 0 and -1 fail startup). Validation runs before any provider is created, and a later build failure disposes already-created providers. A new UpstreamPoolGroupRegistry (interface, empty no-op default and immutable implementation) holds one client per group built by HttpClientFactory through the same configureClient path as the default client, so TLS, proxy, keep-alive, idle handlers, http2 negotiation and the shared loop resources are reused while each group owns a bounded ConnectionProvider; when the strategy is webClient the registry also builds a per-group WebClient with the same codec limits. An optional poolGroup field on the divide rule handle selects a group: DividePlugin clears any stale selection when an exchange is re-entered and publishes the handle value as an exchange attribute, the plugins resolve the group client once per request, and a reference to an unknown group (or a registry that has closed) returns HTTP 503 instead of silently falling back. Because the attribute is written before the request and never rewritten by resend, the group is pinned for the full request lifetime including retries, failover and the response body. The registry owns only the group providers, rejects all lookups after close, and disposes each exactly once even if one dispose fails; with no groups configured every existing path is untouched. Group metrics are opt-in Reactor Netty Micrometer provider meters (metrics: true), tagged with the finite configured group name; the shipped bootstrap carries micrometer-core through actuator, while a lean classpath leaves metrics disabled.

Tests

DivideRuleHandlePoolGroupTest covers handle binding plus equals/hashCode/toString. DefaultUpstreamPoolGroupRegistryTest covers case-insensitive lookups, closed-registry rejection and exactly-once disposal that continues after one provider throws. UpstreamPoolGroupIsolationTest drives two groups against the same loopback origin: a bulk group bounded to one connection saturates while the interactive group on the identical origin keeps serving; an unknown group returns 503 without affecting the default path; failover resolves the selected group for both attempts. HttpClientPoolGroupConfigTest covers startup wiring, empty configuration, duplicate/oversized/blank/omitted/sentinel names, non-FIXED and unbounded limits, the webClient strategy, and custom-client rejection. HttpClientGroupRegistryRollbackTest proves validation precedes provider creation and a partial build disposes what was already built. HttpClientPoolGroupMetricsTest asserts the group provider meters and name tag with a real loopback request. DividePluginPoolGroupTest covers handle-to-attribute publication and re-entered exchanges dropping or replacing the previous group. Removing the group routing turns the isolation and re-entry cases red.

Verification

mvn -pl shenyu-common,shenyu-plugin/shenyu-plugin-httpclient,shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide,shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-httpclient -am test -DskipTests=false passes 1042 tests (666 + 174 + 162 + 40); checkstyle is clean on all four modules. Configuration, the FIXED-only/name/bound rules, per-gateway/per-origin scope, 503 behavior, group metrics (including the Micrometer-vs-ShenYu-Prometheus caveat) and restart-only rollback are documented in docs/upstream-pool-groups.md, with a commented example in the bootstrap application.yml and a release-note migration entry. The dashboard form does not expose poolGroup yet; rules can be configured through the Admin API handle JSON.

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

Stream Agent analysis

FINAL synthesis of PR #8299 (head 9a5d040) adding opt-in named upstream connection-pool groups for workload isolation. All 26 partition reviews passed with no HIGH/CRITICAL defects; complete diff supplied; CI with 64 checks passed. Cross-file assessment from child summaries shows coherent integration of registry, plugin modifications, configuration and tests. No blocking issues found.

Evidence

  • 26/26 partition reviews PASS, no blocking findings (all_parts_passed true)
  • CI passed: 64 checks complete, legacy pending but passed true
  • complete_diff true, global_complete_diff true for head 9a5d040

Recommended next steps

  • Maintainer to perform final code review and merge if satisfied
  • Validate runtime isolation in staging with representative workloads
  • Monitor connection pool metrics tagged by group name post-deploy

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: 09e76a6d-c595-435f-bd70-30ef3ca58d2f · Commit: 9a5d0400dd74ab3f6c3a13bc96f40050d1b3bce5

Review verdict: PASS

SYNTHESIS/FINAL scope: assessed cross-file correctness, compatibility, contracts, tests and interactions from all 26 child partition summaries. Each partition (docs, config, registry, plugins, tests, benchmarks) passed with no HIGH/CRITICAL defects. complete_diff and global_complete_diff true, CI passed (64 checks). No child used NEEDS_CHANGES or INCONCLUSIVE. Therefore FINAL PASS is warranted. Limitations: no local test execution; assessment relies on supplied partition reviews and CI status.

Publication: APPROVE (EXPLICIT_PASS)

Review details

This PR introduces opt-in named upstream connection-pool groups for workload isolation. The cross-file review of all 26 partitions (complete diff) found no concrete blocking defects. CI passed (64 checks). The feature includes documentation, configuration constants, registry implementations, plugin wiring, and comprehensive tests (unit, integration, benchmark). Based on the supplied evidence, I recommend merging after standard maintainer review. Note: I have not executed tests locally; this assessment is based on provided partition reviews and CI status.

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

Stream Agent analysis

FINAL synthesis of PR #8299 (head 9a5d040) adding opt-in named upstream connection-pool groups for workload isolation. All 26 partition reviews passed with no HIGH/CRITICAL defects; complete diff supplied; CI with 64 checks passed. Cross-file assessment from child summaries shows coherent integration of registry, plugin modifications, configuration and tests. No blocking issues found.

Evidence

  • 26/26 partition reviews PASS, no blocking findings (all_parts_passed true)
  • CI passed: 64 checks complete, legacy pending but passed true
  • complete_diff true, global_complete_diff true for head 9a5d040

Recommended next steps

  • Maintainer to perform final code review and merge if satisfied
  • Validate runtime isolation in staging with representative workloads
  • Monitor connection pool metrics tagged by group name post-deploy

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: 09e76a6d-c595-435f-bd70-30ef3ca58d2f · Commit: 9a5d0400dd74ab3f6c3a13bc96f40050d1b3bce5

Review verdict: PASS

SYNTHESIS/FINAL scope: assessed cross-file correctness, compatibility, contracts, tests and interactions from all 26 child partition summaries. Each partition (docs, config, registry, plugins, tests, benchmarks) passed with no HIGH/CRITICAL defects. complete_diff and global_complete_diff true, CI passed (64 checks). No child used NEEDS_CHANGES or INCONCLUSIVE. Therefore FINAL PASS is warranted. Limitations: no local test execution; assessment relies on supplied partition reviews and CI status.

Publication: COMMENT (CI_NOT_PASSED)

Review details

This PR introduces opt-in named upstream connection-pool groups for workload isolation. The cross-file review of all 26 partitions (complete diff) found no concrete blocking defects. CI passed (64 checks). The feature includes documentation, configuration constants, registry implementations, plugin wiring, and comprehensive tests (unit, integration, benchmark). Based on the supplied evidence, I recommend merging after standard maintainer review. Note: I have not executed tests locally; this assessment is based on provided partition reviews and CI status.

@Aias00
Aias00 merged commit c5ff59f into apache:master Nov 10, 2026
64 checks passed
@Aias00 Aias00 added this to the 2.7.3 milestone Nov 13, 2026
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.

[Feature][HTTP] Add opt-in named upstream connection-pool groups for workload isolation

2 participants