You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@shenyu.apache.org by "ww783 (via GitHub)" <gi...@apache.org> on 2026/04/03 15:30:09 UTC

[PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

ww783 opened a new pull request, #6317:
URL: https://github.com/apache/shenyu/pull/6317

   ## Summary
   
   * add gray release fields to `DivideRuleHandle`
   * add `GrayLoadBalancer` for gray routing and traffic splitting
   * add metadata support for divide discovery upstreams
   * preserve admin-managed `gray` and `metadata` during discovery upstream updates
   
   ## Migration
   
   * existing 2.7.1 deployments need to run the new `2.7.1-upgrade-2.7.2-*` script
     before 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 compile`
   
   Ref #6071


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.

Aias00 commented on PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#issuecomment-6110426584

   Hi @ww783, this PR has been in `Changes Requested` since my earlier review and there has been no follow-up push addressing the requested fixes (last commit 2026-04-03).
   
   To keep the review queue healthy, please either:
   - (a) push the requested changes and request a re-review, or
   - (b) rebase onto the latest `master` if it has diverged.
   
   If there are no plans to continue, we may close it for staleness — happy to reopen if you pick it up later. Thanks!
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "yu199195 (via GitHub)" <gi...@apache.org>.

yu199195 commented on PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#issuecomment-4417427448

   Hi， can you add my wechat： xixy199195 ？


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.

Copilot commented on code in PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#discussion_r3039637213


##########
shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java:
##########
@@ -97,4 +101,30 @@ public void handlerDiscoveryUpstreamDataTest() {
     public void pluginNamedTest() {
         assertEquals(divideUpstreamDataHandler.pluginName(), PluginEnum.DIVIDE.getName());
     }
+
+    @Test
+    public void testMetadataFromColumn() {
+        DiscoveryUpstreamData upstream = DiscoveryUpstreamData.builder()
+                .url("127.0.0.1:8080")
+                .protocol("http://")
+                .status(0)
+                .weight(50)
+                .props("{\"warmup\":\"10\",\"gray\":\"true\"}")
+                .metadata("{\"customTag\":\"canary\",\"region\":\"us-east\"}")
+                .dateUpdated(new Timestamp(System.currentTimeMillis()))
+                .build();
+        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"));
+    }

Review 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`.



##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java:
##########
@@ -85,6 +88,9 @@ public CommonUpstream mapToCommonUpstream(DiscoveryUpstreamData discoveryUpstrea
                     .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;

Review 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.



##########
shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java:
##########
@@ -69,6 +65,10 @@ private List<Upstream> convertUpstreamList(final List<DiscoveryUpstreamData> ups
         }
         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());

Review 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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "damingqicai (via GitHub)" <gi...@apache.org>.

damingqicai commented on PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#issuecomment-4438491080

   如何取消订阅，辛苦回复一下，谢谢
   
   
   
   
   
   
   大明奇才
   ***@***.***
   
   
   
           
   
   
   
            原始邮件
            
          
   发件人：xiaoyu ***@***.***&gt;
   发件时间：2026年5月11日 11:40
   收件人：apache/shenyu ***@***.***&gt;
   抄送：Subscribed ***@***.***&gt;
   主题：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&nbsp;or Android. 
   You are receiving this because you are subscribed to this thread.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.

Aias00 closed pull request #6317: [ISSUE #6071] Add gray release support for divide plugin
URL: https://github.com/apache/shenyu/pull/6317


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.

Aias00 commented on PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#issuecomment-6117397599

   Hi @ww783, this PR currently has merge conflicts with the `master` base branch, so CI cannot validate it against the latest code. Please rebase/merge the latest `master` and resolve the conflicts, then push the updated branch. Once the conflicts are cleared, CI will re-run automatically and we can continue the review. Thanks!
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ISSUE #6071] Add gray release support for divide plugin [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.

Aias00 commented on PR #6317:
URL: https://github.com/apache/shenyu/pull/6317#issuecomment-6358767338

   Closing this as stale, per the repository cleanup of long-inactive pull requests.
   
   Status: the last commit is from 2026-06-22, more than three months ago, the review requested changes, and the branch no longer merges into master. Gray release support for the divide plugin is a sizeable feature, so it is better restarted against the current plugin and load-balancer code than carried forward from a stale base.
   
   Please reopen or open a fresh PR when you have time to continue; the review thread here is a good starting point.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org