apache / shenyu

Apache ShenYu is a Java native API Gateway for service proxy, protocol conversion and API governance.
https://shenyu.apache.org/
Apache License 2.0
8.44k stars 2.93k forks source link

[Question] How about change ribbon `ServerListRefreshInterval` from 30s to 10s ? #2916

Closed loongs-zhang closed 2 years ago

loongs-zhang commented 2 years ago

Question

If the data of the registry (such as zookeeper) has been updated, SpringCloudPlugin will still select unavailable instances when using Ribbon for load balancing, change ServerListRefreshInterval from 30s to 10s can't completely solve this problem, but it can be avoided to a great extent.

loongs-zhang commented 2 years ago

Oh, maybe it's better to refactor like DividePlugin ?

发自我的iPhone

------------------ Original ------------------ From: dragon-zhang @.> Date: Mon,Feb 21,2022 5:45 PM To: apache/incubator-shenyu @.> Cc: dragon-zhang @.>, Your activity @.> Subject: Re: [apache/incubator-shenyu] [Question] How about change ribbon ServerListRefreshInterval from 30s to 10s ? (Issue #2916)

Question

If the data of the registry (such as zookeeper) has been updated, SpringCloudPlugin will still select unavailable instances when using Ribbon for load balancing, change ServerListRefreshInterval from 30s to 10s can't completely solve this problem, but it can be avoided to a great extent.

— 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.Message ID: @.***>

yu199195 commented 2 years ago

this props,maybe can dynamic config~

loongs-zhang commented 2 years ago

In ShenyuClientRegisterSpringCloudServiceImpl#buildHandle, I found that divideUpstreams has been set in the handle field of the selector, which means that we can do this directly in SpringCloudPlugin#doExecute:

    @Override
    protected Mono<Void> doExecute(final ServerWebExchange exchange, final ShenyuPluginChain chain, final SelectorData selector,
                                   final RuleData rule) {
        if (Objects.isNull(rule)) {
            return Mono.empty();
        }
        final ShenyuContext shenyuContext = exchange.getAttribute(Constants.CONTEXT);
        assert shenyuContext != null;
        final SpringCloudSelectorHandle springCloudSelectorHandle = SpringCloudPluginDataHandler.SELECTOR_CACHED.get().obtainHandle(selector.getId());
        final SpringCloudRuleHandle ruleHandle = SpringCloudPluginDataHandler.RULE_CACHED.get().obtainHandle(CacheKeyUtils.INST.getKey(rule));

        //Below is a partial implementation of the code I plan to refactor
        List<Upstream> upstreamList = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId(selector.getId());
        //......
    }
loongs-zhang commented 2 years ago

In this way, another benefit is that we don't have to depend on Ribbon.

loongs-zhang commented 2 years ago

In ShenyuClientRegisterSpringCloudServiceImpl#buildHandle, I found that divideUpstreams has been set in the handle field of the selector, which means that we can do this directly in SpringCloudPlugin#doExecute:

    @Override
    protected Mono<Void> doExecute(final ServerWebExchange exchange, final ShenyuPluginChain chain, final SelectorData selector,
                                   final RuleData rule) {
        if (Objects.isNull(rule)) {
            return Mono.empty();
        }
        final ShenyuContext shenyuContext = exchange.getAttribute(Constants.CONTEXT);
        assert shenyuContext != null;
        final SpringCloudSelectorHandle springCloudSelectorHandle = SpringCloudPluginDataHandler.SELECTOR_CACHED.get().obtainHandle(selector.getId());
        final SpringCloudRuleHandle ruleHandle = SpringCloudPluginDataHandler.RULE_CACHED.get().obtainHandle(CacheKeyUtils.INST.getKey(rule));

        //Below is a partial implementation of the code I plan to refactor
        List<Upstream> upstreamList = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId(selector.getId());
        //......
    }

Haha, as I supposed :)

111111_3c411993-a2c3-419a-96f3-1077dc28d722
loongs-zhang commented 2 years ago

this props,maybe can dynamic config~

I think List<Upstream> upstreamList = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId(selector.getId()); could be better, what do you think ?

yu199195 commented 2 years ago

good pr, i will close this issue~