[yangjj-iso]

Fixes #8019.

Stable and canary requests currently fail over through the legacy gray-exclusive pool, which can route a retry outside the selected label partition. Capture immutable, selector-scoped labels after Divide resolves its initial partition or fallback, then apply those labels to the latest healthy full snapshot before each failover selection. An exhausted partition fails with 503; a backend failure does not select a new fallback. Legacy rules retain their existing gray-exclusive pool.

Validation on OpenJDK 17.0.20:

• All 129 selected Divide/HTTP retry tests pass, with zero failures/errors/skips.
• Twelve new cross-module regressions exercise actual Divide execution and discovery snapshots: stable/canary partitions, initial stable fallback, legacy gray nodes, topology refresh, rule-setting changes, exhausted partitions, default/fixed FAILOVER, and default/fixed/exponential CURRENT behavior.
• Removing the production fix reproduces six regression failures; restoring it passes.
• Affected-reactor compilation, Checkstyle, root Apache RAT and git diff --check pass.

Full-repository and Docker E2E suites were not run. Local Mockito tests use the existing Byte Buddy agent because self-attachment is unavailable.

• Submit regression tests.
• Full ./mvnw clean install -Dmaven.javadoc.skip=true (targeted validation is recorded above).

[Aias00]

Review (Aias00)

Verdict: APPROVE. The bug is real, the fix is scoped to the failover lookup, and the new suite fails 6/12 when the retry-path change is reverted, so it is pinned rather than decorative.

What I verified

• mvn -pl shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide,shenyu-plugin/shenyu-plugin-httpclient -am test: 234 tests, 0 failures, 0 errors, 0 skipped (divide 114, httpclient 120), including DivideCanaryFailoverTest 12/12.
• Reverted only the resend() lookup hunk (leaving everything else, including the new context class, in place) and re-ran: Tests run: 12, Failures: 6 — all four failoverRetainsResolvedPartitionAfterRefresh variants plus both exhaustedPartitionCannotFallBackAfterBackendFailure cases. The six that fail are exactly the ones that assert partition retention, which is the behavior under change.
• Checked the identity assumption that the whole fix depends on: UpstreamRoutingContext is built with selector.getId() in DividePlugin.selectPartition, and the retry path compares it against Constants.DIVIDE_SELECTOR_ID, which DividePlugin:212 sets from selector.getId() as well. Same source, so filter cannot reject a legitimate retry.
• Checked the health semantics of the switch from findUpstreamListBySelectorId to findAllUpstreamListBySelectorId + label filter. findAll... reads task.getHealthyUpstream(), i.e. still the healthy snapshot; the only difference is that it does not apply the legacy gray-exclusive projection. That is exactly what the fix needs — the legacy projection is what could route a retry outside the selected label partition — and health filtering is not lost.
• Confirmed the empty-result path is safe: filter returns an empty list rather than null on a selector mismatch, and resend() already maps an empty candidate list to ShenyuException → 503 CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER. No NPE, no silent retry against a stale URI.

Design notes

• Capturing the labels after the canary decision, including the fallback branch, is the correct place: the fallback is itself a resolved decision, and re-deciding on retry is precisely what produced the cross-partition retry. The test that mutates the shared CanaryConfig mid-request (config.setStableLabels(Map.of("release", "changed"))) is a good way to prove the request-scoped copy is used instead of the live config — that is the failure mode a naive implementation would have.
• UpstreamRoutingContext.filter carrying the selector identity is the right guard. Without it, an exchange re-entered against a different selector could silently inherit a stale partition.
• Clearing the attribute at the top of executeRequest is defensive and correct for a re-used exchange; it also makes the "no canary decision happened" case distinguishable in legacyFailoverKeepsGrayExclusivePool, which asserts the attribute is null and the gray-exclusive pool still applies. That legacy-preservation assertion is the one I most wanted to see, since the easy version of this fix would have changed legacy behavior too.

Nits (non-blocking)

1. The pom.xml change promotes shenyu-plugin-httpclient from the divide-gateway-benchmark profile into the module's default test scope. It works and there is no cycle (httpclient does not depend on divide), but it now resolves for every divide build, including plain -DskipTests compiles, purely because one new test needs DefaultRetryStrategy. Consider either keeping it in the profile and adding that profile to the CI invocation, or a one-line comment in the POM saying why the scope moved — the next person to trim test dependencies will otherwise try to put it back.
2. UpstreamRoutingContext.filter both filters by labels and enforces selector identity. The name reads as "apply the partition"; filterFor(selectorId, upstreams) or a short javadoc sentence on the identity check would make the second job visible at the call site, where it currently looks like a plain label filter.
3. DividePlugin.executeRequest removing the attribute at entry is not self-explanatory. A comment like "a re-entered exchange must not inherit a previous selector's partition" would stop someone from deleting it as dead code.
4. exhaustedPartitionCannotFallBackAfterBackendFailure asserts http.attempts.isEmpty(), which is the right assertion, but it does not distinguish "the retry never ran" from "the retry ran and selected nothing before touching the client". Given the 503 assertion above it, the distinction is covered in practice — just noting the assertion is doing less than it appears to.

Interaction with #8035 / #8078

No overlap in files with the gRPC work; this is divide + httpclient only. Independent to merge.

CI

Green on 07d6e02c: build, changes, pr_build, rat_check, check-license-header, registry_dependencies, build (shenyu-integrated-test-http), build (shenyu-integrated-test-k8s-gateway-api-http), CodeQL Analyze (java), e2e all pass.

[Aias00]

Approved on the technical merits, but please do not merge this before it is coordinated with #8088 — that PR implements the same fix (same DefaultRetryStrategy.resend call site, same DividePlugin partition snapshot, same exchange-attribute carrier) from a different author. Only one of the two can land, and whichever loses should be closed rather than rebased. I have flagged the same thing over there.

Verified locally

• shenyu-plugin-divide 114/114 and shenyu-plugin-httpclient 120/120 green; checkstyle:check clean on divide, httpclient and base.
• Rollback: reverting only the two integration points (DividePlugin.selectPartition, DefaultRetryStrategy.resend) while keeping UpstreamRoutingContext fails 6 of the 12 new cases, and the failure messages are exactly the bug: expected: <[http://second:8080/test]> but was: <[http://opposite:8080/test]> — the retry escaped into the other partition — plus expectErrorSatisfies ... actual: onComplete() for the exhausted-partition case, which used to silently succeed instead of returning 503. The non-discriminating parameterisations (stable/default, non-canary) stay green, so the test is honest about what it pins.
• I checked the health semantics that the retry path relies on: findAllUpstreamListBySelectorId returns task.getHealthyUpstream() (healthy only), and findUpstreamListBySelectorId is that plus the legacy gray-exclusive filter. So filtering the all list by the resolved labels keeps health filtering intact and only drops the legacy gray restriction — which is correct here, because DividePlugin already resolves its canary pool from the all-list.
• UpstreamRoutingContext copies the label map into a new HashMap and wraps it unmodifiable, so the mid-flight config.setStableLabels(...) mutation in failoverRetainsResolvedPartitionAfterRefresh really is testing snapshot semantics and not accidentally passing. Good test design.
• DIVIDE_SELECTOR_ID is written with selector.getId() (DividePlugin:213) and the context is created with the same value, so the filter guard cannot false-negative on the normal path.

Non-blocking

1. When UpstreamRoutingContext.filter rejects a selector mismatch it returns Collections.emptyList(), which the caller reports as the generic CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER. The pool was healthy; the selector just changed under the request. A distinct message would make that traceable in production.
2. No RELEASE-NOTES.md entry, while other recently merged behaviour changes (#8043) added one. This changes retry routing for canary rules, so it is worth a line.
3. Minor: shenyu-plugin-httpclient moved from the profile block to the top-level test dependency block. Harmless (test scope) but it now applies to every build of the module — worth a word in the PR description so reviewers do not wonder why divide tests need the HTTP client.

Nice catch on removing the attribute at the top of executeRequest; re-entered exchanges inheriting a stale partition would have been an ugly follow-up bug.

[Aias00]

Returning an empty list on selector mismatch makes the caller fall through to CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER, which claims the pool has no healthy upstream. That is not what happened here — the pool was fine, the selector changed underneath the request.

Since this is the only branch that can produce it, consider making the mismatch visible (a debug log with both selector ids, or a dedicated message on the caller side) so an operator debugging a 503 does not chase a non-existent health problem.