[ISSUE #6071] Add gray release support for divide plugin - #6317
Conversation
* add gray release fields to DivideRuleHandle and a dedicated GrayLoadBalancer * add metadata support for divide discovery upstreams * preserve gray and metadata during discovery upstream sync and updates
There was a problem hiding this comment.
Pull request overview
Adds gray (canary) release routing support to the Divide plugin by propagating upstream metadata/gray flags through admin → discovery sync → gateway cache, and introducing a dedicated GrayLoadBalancer that can route based on request attributes and traffic percentage.
Changes:
- Extend divide rule handle / discovery upstream models to carry gray-routing configuration and upstream metadata.
- Add
GrayLoadBalancerSPI implementation and wire Divide plugin to use it when gray is enabled. - Persist and synchronize
metadatafor discovery upstreams across admin DB schema, mappings, and gateway discovery handling.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shenyu-web/src/main/java/org/apache/shenyu/web/controller/LocalPluginController.java | Include gray and metadata when saving discovery upstream data for sync. |
| shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java | Add test asserting upstream gray/metadata parsing from discovery columns. |
| shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DividePluginTest.java | Add tests for gray-mode attribute/config exposure and callback behavior; tighten static mocking scope. |
| shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java | Always submit full upstream list; parse and attach metadata into Upstream. |
| shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java | Switch load-balance algorithm to gray when enabled and attach GrayConfig into exchange attributes. |
| shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/spi/GrayLoadBalancerTest.java | New unit tests covering condition types/operators, percent routing, metadata match, and fallbacks. |
| shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java | Add test ensuring gray/metadata are synced when updating an existing upstream entry. |
| shenyu-loadbalancer/src/main/resources/META-INF/shenyu/org.apache.shenyu.loadbalancer.spi.LoadBalancer | Register gray load balancer SPI implementation. |
| shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/spi/GrayLoadBalancer.java | New load balancer that performs gray routing based on request attributes + traffic split + metadata filter. |
| shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java | Add builder support and defensive copying for metadata. |
| shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java | Ensure updates to existing upstreams also sync gray and metadata. |
| shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandleTest.java | Extend tests to cover new gray fields and Gson deserialization. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/DiscoveryUpstreamData.java | Add metadata field to discovery upstream DTO and include it in equality/hash. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/selector/CommonUpstream.java | Add metadata map to upstream model used during conversion/sync. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/MetadataMatch.java | New DTO for matching upstream metadata key/value. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandle.java | Add gray-routing configuration fields into Divide rule handle. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/GrayConfig.java | New DTO holding gray routing conditions, percent, delegate LB, and metadata match. |
| shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/GrayCondition.java | New DTO describing request-attribute-based gray activation conditions. |
| shenyu-admin/src/main/resources/sql-script/h2/schema.sql | Add metadata column to discovery_upstream (H2). |
| shenyu-admin/src/main/resources/mappers/discovery-upstream-sqlmap.xml | Map/persist metadata for discovery upstreams in MyBatis. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/CommonUpstreamUtils.java | Propagate metadata when converting to CommonUpstream. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java | Transfer metadata between DO/DTO/VO and parse JSON into CommonUpstream metadata map. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java | Persist discovery upstream metadata on add/update. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/model/vo/DiscoveryUpstreamVO.java | Expose metadata in discovery upstream view object. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/DiscoveryUpstreamDO.java | Store metadata in discovery upstream entity and builder. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/ProxySelectorAddDTO.java | Accept metadata on discovery upstream payload. |
| shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/DiscoveryUpstreamDTO.java | Add metadata to discovery upstream DTO. |
| db/upgrade/2.7.1-upgrade-2.7.2-pg.sql | Add metadata column upgrade for PostgreSQL. |
| db/upgrade/2.7.1-upgrade-2.7.2-oracle.sql | Add metadata column upgrade for Oracle. |
| db/upgrade/2.7.1-upgrade-2.7.2-og.sql | Add metadata column upgrade for openGauss-compatible script. |
| db/upgrade/2.7.1-upgrade-2.7.2-ob.sql | Add metadata column upgrade for OceanBase. |
| db/upgrade/2.7.1-upgrade-2.7.2-mysql.sql | Add metadata column upgrade for MySQL. |
| db/init/pg/create-table.sql | Add metadata column to PostgreSQL init schema. |
| db/init/oracle/schema.sql | Add metadata column and comment to Oracle init schema. |
| db/init/og/create-table.sql | Add metadata column to openGauss init schema. |
| db/init/ob/schema.sql | Add metadata column to OceanBase init schema. |
| db/init/mysql/schema.sql | Add metadata column to MySQL init schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .orElse(new Properties()); | ||
| commonUpstream | ||
| .setHealthCheckEnabled(Boolean.parseBoolean(properties.getProperty("healthCheckEnabled", "true"))); | ||
| commonUpstream.setMetadata(StringUtils.isBlank(data.getMetadata()) | ||
| ? Collections.emptyMap() | ||
| : GsonUtils.getInstance().toObjectMap(data.getMetadata(), String.class)); | ||
| return commonUpstream; |
There was a problem hiding this comment.
DiscoveryTransfer#mapToCommonUpstream parses data.getMetadata() with GsonUtils.toObjectMap(...) without guarding against invalid JSON. Since metadata is user-provided text, a malformed value can throw at runtime and break discovery/upstream sync. Consider validating metadata as JSON on input (DTO validation) and/or catching JSON parse errors here and falling back to an empty map.
| return upstreamList.stream().map(u -> { | ||
| Properties properties = Optional.ofNullable(u.getProps()).map(ps -> GsonUtils.getInstance().fromJson(ps, Properties.class)).orElse(new Properties()); | ||
| Map<String, String> metadata = Optional.ofNullable(u.getMetadata()) | ||
| .filter(value -> !value.isBlank()) | ||
| .map(value -> GsonUtils.getInstance().toObjectMap(value, String.class)) | ||
| .orElse(Collections.emptyMap()); |
There was a problem hiding this comment.
DivideUpstreamDataHandler#convertUpstreamList deserializes u.getMetadata() via GsonUtils.toObjectMap(...) without handling invalid JSON. Because metadata ultimately originates from persisted text, a malformed value will throw and abort upstream updates for the selector. Add defensive parsing (catch JSON syntax errors / validate before parsing) and default to an empty map when parsing fails.
| DiscoverySyncData syncData = new DiscoverySyncData(); | ||
| syncData.setSelectorId("metadata-test"); | ||
| syncData.setUpstreamDataList(Collections.singletonList(upstream)); | ||
| syncData.setPluginName("divide"); | ||
|
|
||
| divideUpstreamDataHandler.handlerDiscoveryUpstreamData(syncData); | ||
| List<Upstream> result = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("metadata-test"); | ||
|
|
||
| assertFalse(result.isEmpty()); | ||
| assertTrue(result.get(0).isGray()); | ||
| Map<String, String> metadata = result.get(0).getMetadata(); | ||
| assertEquals("canary", metadata.get("customTag")); | ||
| assertEquals("us-east", metadata.get("region")); | ||
| } |
There was a problem hiding this comment.
testMetadataFromColumn writes into the singleton UpstreamCacheManager under selectorId metadata-test but never removes it. This can leak state across tests and create ordering-dependent failures when tests share a JVM. Clean up the selector entry (e.g., removeByKey) in the test or in @AfterEach.
Guard metadata deserialization with safe fallbacks and clear UpstreamCacheManager entries after tests to avoid state leakage.
|
Hi， can you add my wechat： xixy199195 ？ |
|
如何取消订阅，辛苦回复一下，谢谢
大明奇才
***@***.***
原始邮件
发件人：xiaoyu ***@***.***>
发件时间：2026年5月11日 11:40
收件人：apache/shenyu ***@***.***>
抄送：Subscribed ***@***.***>
主题：Re: [apache/shenyu] [ISSUE #6071] Add gray release support for divide plugin (PR #6317)
yu199195 left a comment (apache/shenyu#6317)
Hi， can you add my wechat： xixy199195 ？
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
|
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Dual-lane review (code-reviewer + architect) of #6317 (head 7e54812c). Requesting changes.
High — regresses the #6412 cache-pollution fix. DividePlugin.doExecute still does upstream.setUrl(specifyDomains.get(0)) after selecting from UpstreamCacheManager (cached, shared instance), so a Specify-Domain header permanently overwrites a cached upstream's URL for all subsequent requests. Master commit d4db5e60d (#6412, merged 2026-07-05) replaced exactly this with filter-first + rebuild-new-Upstream. This PR (last commit 2026-06-22) predates #6412, so it will regress that fix unless rebased. Please rebase onto current master and reconcile doExecute with #6412's handling, and decide/document the gray-vs-specifyDomain precedence (today gray runs on the full pool and the selected URL is then overwritten, so Specify-Domain silently bypasses gray routing). Add a gray + specifyDomain test.
High (architect) — silent semantic regression of the existing props.gray exclusive-pool behavior. Master's DivideUpstreamDataHandler (from #5763) submits gray upstreams as an exclusive pool when any props.gray=true exists. This PR removes that branch and always submits a mixed pool; gray upstreams only matter when the new rule handle sets grayEnabled=true. Any deployment relying on the existing gray flag (e.g. a custom discovery provider injecting gray:true into props) will silently start sending production traffic to gray upstreams under the normal LB, with no migration note. For a non-breaking 2.7.1→2.7.2, please add a compat fallback (preserve exclusive-pool behavior when grayEnabled=false but gray upstreams exist) or an explicit behavior-change note.
Medium — matchMetadata matches ALL gray upstreams when value is null. GrayLoadBalancer.matchMetadata returns true when metadataMatch.getValue() is null, and DividePlugin.buildGrayConfig builds a MetadataMatch whenever grayMetadataKey is set (value defaults to null). So a rule with the metadata key set but value blank partitions against every gray upstream, defeating version partitioning. Treat a blank value as "no metadata filter" (skip) or require both key and value at config time.
Medium — pure percent-based canary is impossible. shouldRouteToGray returns false when conditions is empty before consulting percent, and grayPercent defaults to 0, so grayEnabled=true + grayPercent=50 with no condition routes 0% to gray with no warning. Either allow percent-only routing or log a warning when gray is enabled without a condition/percent>0.
Architect design items (non-blocking but please address):
- Dual gray-identity model.
GrayLoadBalancerrequiresUpstream.isGray()==trueand a metadata match; admin now exposes the boolean (via props) and the metadata column independently with no enforced invariant. Partial config silently degrades. Collapse to one identity model — preferably metadata-only partitioning — or deriveisGray()from the configured metadata at sync time. - Abstraction/schema mismatch.
GrayConfigcarriesList<GrayCondition>(OR semantics) butDivideRuleHandleflattens to a single condition andbuildGrayConfigemits a ≤1-element list — the list path is dead and forces a future schema migration. Either make the rule handle aList<GrayCondition>now, or drop the list fromGrayConfig. - Layering. Header/cookie/query/ip condition matching is reimplemented inside
GrayLoadBalancerwith a bespoke operator set, parallel to Shenyu's existing rule/selector match engine. ReuseMatchStrategy/Condition, or reconsider whether gray routing belongs in the LB at all vs. a separate selector+rule pointing at a gray upstream group.
Low. On retry, DefaultRetryStrategy.resend re-enters GrayLoadBalancer.select and re-rolls ThreadLocalRandom.nextInt(100) < percent, so a gray request may fail over to the normal pool — consider caching the route-to-gray decision in the exchange for the request lifetime. Also, gray selection is unit-tested in GrayLoadBalancerTest but not through the real DividePlugin with a mixed gray/normal pool (doExecuteGrayModeShouldExposeGrayConfigAndLoadBalanceAttribute only asserts the attribute is set, using non-gray-flagged upstreams) — a plugin-level test asserting the gray upstream is actually selected from a mixed pool would guard the wiring.
Positives: defensive copying in Upstream.setMetadata/Builder, fail-closed matchCondition try/catch, NULL-metadata compatibility in DiscoveryTransfer/DivideUpstreamDataHandler, and the toString comma fix. The retry path correctly re-uses the persisted GrayConfig exchange attribute.
Summary
DivideRuleHandleGrayLoadBalancerfor gray routing and traffic splittinggrayandmetadataduring discovery upstream updatesMigration
2.7.1-upgrade-2.7.2-*scriptbefore starting the updated admin image
Testing
./mvnw -pl shenyu-common,shenyu-loadbalancer,shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide -am -DskipITs -DfailIfNoTests=false -Dtest=DivideRuleHandleTest,PluginEnumTest,UpstreamCacheManagerTest,GrayLoadBalancerTest,DividePluginTest,DivideUpstreamDataHandlerTest test -Dmaven.javadoc.skip=true./mvnw -pl shenyu-admin -am -DskipTests -Dmaven.javadoc.skip=true compile./mvnw -pl shenyu-web -am -DskipTests -Dmaven.javadoc.skip=true compileRef #6071