You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/09/30 03:41:01 UTC

[GitHub] [dubbo] HelloWorld-97 opened a new pull request #8967: Dubbo3.0 :adaptive service

HelloWorld-97 opened a new pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967


   ## What is the purpose of the change
   A adaptive cluster scheduling mechanism
   server: Based on congestion control algorithm of TCP/IP Protocol
   client: Based on P2C loadBalance
   
   ## Brief changelog
   
   
   ## Verifying this change
   
   
   <!-- Follow this checklist to help us incorporate your contribution quickly and easily: -->
   
   ## Checklist
   - [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
   - [ ] Each commit in the pull request should have a meaningful subject line and body.
   - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
   - [ ] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
   - [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project.
   - [ ] Add some description to [dubbo-website](https://github.com/apache/dubbo-website) project if you are requesting to add a feature.
   - [ ] GitHub Actions works fine on your own branch.
   - [ ] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/dubbo/wiki/Software-donation-guide).
   


-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] justxuewei commented on a change in pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
justxuewei commented on a change in pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967#discussion_r740278545



##########
File path: dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/limiter/AbstractLimiter.java
##########
@@ -0,0 +1,51 @@
+package org.apache.dubbo.rpc.filter.limiter;
+
+import org.apache.dubbo.rpc.filter.limit.Limit;
+import org.apache.dubbo.rpc.filter.limit.VegasLimit;
+
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
+
+public abstract class AbstractLimiter implements Limiter{
+    private static final Supplier<Long> clock = System::nanoTime;
+    protected Limit limitAlgorithm;
+    protected AtomicInteger inflight;
+
+    public AbstractLimiter(){
+        this.limitAlgorithm = new VegasLimit();

Review comment:
       Why use VegasLimit as default limit algorithm for the AbstractLimiter?




-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] justxuewei commented on a change in pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
justxuewei commented on a change in pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967#discussion_r731502394



##########
File path: dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/AdaptiveFilter.java
##########
@@ -0,0 +1,70 @@
+package org.apache.dubbo.rpc.filter;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.filter.limiter.AbstractLimiter;
+import org.apache.dubbo.rpc.filter.limiter.Limiter;
+import org.apache.dubbo.rpc.filter.limiter.SimpleLimiter;
+
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
+
+/**
+ * 服务端过滤器
+ * 可选接口
+ * 此类可以修改实现,不可以移动类或者修改包名
+ * 用户可以在服务端拦截请求和响应,捕获 rpc 调用时产生、服务端返回的已知异常。
+ */
+@Activate(group = CommonConstants.PROVIDER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final ConcurrentHashMap<String, Limiter> name2limiter = new ConcurrentHashMap<>();

Review comment:
       In this case, name2limiter maps method name to limiter. Method names, however, may conflict with others. For example, interface1 has a method named "methodA", interface2 has another method named "methodA" as well, name2limiter will return a same limiter eventually.




-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] chickenlj commented on pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
chickenlj commented on pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967#issuecomment-1068844119


   Could you please submit a new patch to the extension repo apache/dubbo-spi-extensions?
   
   We can merge it quickly there and make it available as a trial feature. 


-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] guohao commented on a change in pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
guohao commented on a change in pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967#discussion_r719102751



##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/adaptive/Node.java
##########
@@ -0,0 +1,39 @@
+package org.apache.dubbo.rpc.cluster.adaptive;
+
+import org.apache.dubbo.rpc.Invoker;
+
+public class Node {
+    private volatile int remain;
+    private Invoker invoker;
+    private String address;
+
+    public Node(Invoker invoker){
+        this();
+        this.invoker = invoker;
+        this.address = invoker.getUrl().getBackupAddress();

Review comment:
       why use backup address?

##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/adaptive/AdaptiveFilter.java
##########
@@ -0,0 +1,37 @@
+package org.apache.dubbo.rpc.cluster.adaptive;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.cluster.loadbalance.P2CLoadBalance;
+
+import java.util.function.Supplier;
+
+@Activate(group = CommonConstants.CONSUMER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final Supplier<Long> clock = System::nanoTime;
+
+    @Override
+    public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+        try {
+            Result result = invoker.invoke(invocation);
+            return result;
+        } catch (Exception e) {
+            throw e;
+        }
+
+    }
+
+    @Override
+    public void onResponse(Result appResponse, Invoker<?> invoker, Invocation invocation) {
+        String remain = appResponse.getAttachment("remain");
+        String limit = appResponse.getAttachment("limit");
+        long pickTime = (Long)invocation.get("pickTime");
+        P2CLoadBalance.updateNodes(Integer.parseInt(remain),invocation.getMethodName(),invoker.getUrl().getBackupAddress(),clock.get() - pickTime);

Review comment:
       use SPI or pass reference here to reduce static code

##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/adaptive/AdaptiveFilter.java
##########
@@ -0,0 +1,37 @@
+package org.apache.dubbo.rpc.cluster.adaptive;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.cluster.loadbalance.P2CLoadBalance;
+
+import java.util.function.Supplier;
+
+@Activate(group = CommonConstants.CONSUMER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final Supplier<Long> clock = System::nanoTime;
+
+    @Override
+    public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+        try {

Review comment:
       unused try catch

##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/loadbalance/P2CLoadBalance.java
##########
@@ -0,0 +1,101 @@
+package org.apache.dubbo.rpc.cluster.loadbalance;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.LoadBalance;
+import org.apache.dubbo.rpc.cluster.adaptive.Node;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Supplier;
+
+public class P2CLoadBalance implements LoadBalance {
+    private static final ConcurrentHashMap<String, List<Node>> method2Nodes = new ConcurrentHashMap<>(32);
+    private static final Supplier<Long> clock = System::nanoTime;
+    private final ReentrantLock lock = new ReentrantLock();
+
+    @Override
+    public <T> Invoker<T> select(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException {
+        //update method2Nodes
+        updatemethod2Nodes(invokers,invocation);
+        //prepick
+        Node[] prepick = prepick(invocation);
+        //pick
+        Invoker pick = pick(prepick);
+        //set pickTime
+        invocation.put("pickTime",clock.get());
+        return pick;
+    }
+
+    private <T> void updatemethod2Nodes(List<Invoker<T>> invokers,Invocation invocation){
+        //todo 注册中心的健康检查机制
+        String methodName = invocation.getMethodName();
+        if (method2Nodes.containsKey(methodName))return;
+        try{
+            lock.lock();
+            method2Nodes.computeIfAbsent(methodName,k-> new ArrayList<>());

Review comment:
       will it be better to use `set` for lookup ?

##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/adaptive/AdaptiveFilter.java
##########
@@ -0,0 +1,37 @@
+package org.apache.dubbo.rpc.cluster.adaptive;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.cluster.loadbalance.P2CLoadBalance;
+
+import java.util.function.Supplier;
+
+@Activate(group = CommonConstants.CONSUMER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final Supplier<Long> clock = System::nanoTime;
+
+    @Override
+    public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
+        try {
+            Result result = invoker.invoke(invocation);
+            return result;
+        } catch (Exception e) {
+            throw e;
+        }
+
+    }
+
+    @Override
+    public void onResponse(Result appResponse, Invoker<?> invoker, Invocation invocation) {
+        String remain = appResponse.getAttachment("remain");

Review comment:
       try use constant instead of literal

##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/loadbalance/P2CLoadBalance.java
##########
@@ -0,0 +1,101 @@
+package org.apache.dubbo.rpc.cluster.loadbalance;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.LoadBalance;
+import org.apache.dubbo.rpc.cluster.adaptive.Node;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Supplier;
+
+public class P2CLoadBalance implements LoadBalance {
+    private static final ConcurrentHashMap<String, List<Node>> method2Nodes = new ConcurrentHashMap<>(32);
+    private static final Supplier<Long> clock = System::nanoTime;
+    private final ReentrantLock lock = new ReentrantLock();
+
+    @Override
+    public <T> Invoker<T> select(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException {
+        //update method2Nodes
+        updatemethod2Nodes(invokers,invocation);
+        //prepick
+        Node[] prepick = prepick(invocation);
+        //pick
+        Invoker pick = pick(prepick);
+        //set pickTime
+        invocation.put("pickTime",clock.get());
+        return pick;
+    }
+
+    private <T> void updatemethod2Nodes(List<Invoker<T>> invokers,Invocation invocation){
+        //todo 注册中心的健康检查机制
+        String methodName = invocation.getMethodName();
+        if (method2Nodes.containsKey(methodName))return;

Review comment:
       brace

##########
File path: dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/AdaptiveFilter.java
##########
@@ -0,0 +1,70 @@
+package org.apache.dubbo.rpc.filter;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.filter.limiter.AbstractLimiter;
+import org.apache.dubbo.rpc.filter.limiter.Limiter;
+import org.apache.dubbo.rpc.filter.limiter.SimpleLimiter;
+
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
+
+/**
+ * 服务端过滤器
+ * 可选接口
+ * 此类可以修改实现,不可以移动类或者修改包名
+ * 用户可以在服务端拦截请求和响应,捕获 rpc 调用时产生、服务端返回的已知异常。
+ */
+@Activate(group = CommonConstants.PROVIDER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final ConcurrentHashMap<String, Limiter> name2limiter = new ConcurrentHashMap<>();

Review comment:
       Is it accessary to use static?




-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] justxuewei commented on a change in pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
justxuewei commented on a change in pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967#discussion_r731502394



##########
File path: dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/AdaptiveFilter.java
##########
@@ -0,0 +1,70 @@
+package org.apache.dubbo.rpc.filter;
+
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.extension.Activate;
+import org.apache.dubbo.rpc.*;
+import org.apache.dubbo.rpc.filter.limiter.AbstractLimiter;
+import org.apache.dubbo.rpc.filter.limiter.Limiter;
+import org.apache.dubbo.rpc.filter.limiter.SimpleLimiter;
+
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
+
+/**
+ * 服务端过滤器
+ * 可选接口
+ * 此类可以修改实现,不可以移动类或者修改包名
+ * 用户可以在服务端拦截请求和响应,捕获 rpc 调用时产生、服务端返回的已知异常。
+ */
+@Activate(group = CommonConstants.PROVIDER)
+public class AdaptiveFilter implements Filter, BaseFilter.Listener {
+    private static final ConcurrentHashMap<String, Limiter> name2limiter = new ConcurrentHashMap<>();

Review comment:
       In this case, the name2limiter maps a method name to a limiter. Method names, however, may conflict with others. For example, interface1 has a method named "methodA", interface2 has another method named "methodA" as well, and then, the name2limiter will return the same limiter for both methods. This is not acceptable.




-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo] chickenlj closed pull request #8967: Dubbo3.0 :adaptive service

Posted by GitBox <gi...@apache.org>.
chickenlj closed pull request #8967:
URL: https://github.com/apache/dubbo/pull/8967


   


-- 
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@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org