[GitHub] [dubbo] guohao commented on a change in pull request #9057: [3.0-Triple] Refactor transport state and other code

[GitBox <gi...@apache.org>]

guohao commented on a change in pull request #9057:
URL: https://github.com/apache/dubbo/pull/9057#discussion_r730564516



##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ClientTransportObserver.java
##########
@@ -34,11 +34,13 @@
 public class ClientTransportObserver implements TransportObserver {
     private final AsciiString SCHEME;
     private final ChannelHandlerContext ctx;
-    private final Http2StreamChannel streamChannel;
+    private volatile Http2StreamChannel streamChannel;
     private final ChannelPromise promise;
-    private boolean headerSent = false;
-    private boolean endStreamSent = false;
-    private boolean resetSent = false;
+//    private boolean headerSent = false;

Review comment:
       Remove unused fileds

##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractServerStream.java
##########
@@ -134,7 +137,7 @@ protected RpcInvocation buildInvocation(Metadata metadata) {
         ClassLoader tccl = Thread.currentThread().getContextClassLoader();
         try {
             if (getProviderModel() != null) {
-                ClassLoadUtil.switchContextLoader(getProviderModel().getClassLoader());
+                ClassLoadUtil.switchContextLoader(getProviderModel().getServiceInterfaceClass().getClassLoader());

Review comment:
       What is the difference bwteen `model.getClassLoader()` and service interface classloader ?

##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ServerTransportObserver.java
##########
@@ -28,34 +28,32 @@
 import io.netty.handler.codec.http2.DefaultHttp2ResetFrame;
 import io.netty.handler.codec.http2.Http2Error;
 
-import static io.netty.handler.codec.http.HttpResponseStatus.OK;
-
 public class ServerTransportObserver implements TransportObserver {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(ServerTransportObserver.class);
 
     private final ChannelHandlerContext ctx;
-    private boolean headerSent = false;
-    private boolean resetSent = false;
+//    private boolean headerSent = false;

Review comment:
       Remove these unused code

##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractClientStream.java
##########
@@ -194,4 +211,23 @@ protected void cancelByLocal(Throwable throwable) {
     }
 
 
+    protected abstract class AbstractClientTransport extends AbstractTransportObserver {
+
+        @Override
+        public void onData(byte[] data, boolean endStream) {
+            execute(() -> {
+                final Object resp = deserializeResponse(data);
+                getStreamSubscriber().onNext(resp);
+            });
+        }
+
+//        protected abstract

Review comment:
       Fix

##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryClientStream.java
##########
@@ -40,10 +38,16 @@ protected UnaryClientStream(URL url) {
     }
 
     @Override
-    protected StreamObserver<Object> createStreamObserver() {
-        return new UnaryClientStreamObserverImpl();
+    protected void doOnStartCall() {
+        asStreamObserver().onNext(getRpcInvocation());
+        asStreamObserver().onCompleted();
     }
 
+//    @Override

Review comment:
       Remove these

##########
File path: dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/AbstractClientStream.java
##########
@@ -69,11 +72,40 @@ public static AbstractClientStream newClientStream(URL url, boolean unary) {
         return stream;
     }
 
+    protected RpcInvocation getRpcInvocation() {
+        return (RpcInvocation) getRequest().getData();
+    }
+
+    protected void startCall() {
+        try {
+            doOnStartCall();
+        } catch (Throwable throwable) {
+            asStreamObserver().onError(throwable);

Review comment:
       Client shoulde not send error to server if exception occurred in sending request 




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