[GitHub] [dubbo] CrazyHZM commented on a diff in pull request #10819: feat(wip): Client decoupling of Connection and Netty

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

CrazyHZM commented on code in PR #10819:
URL: https://github.com/apache/dubbo/pull/10819#discussion_r1016392266


##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/PortUnificationExchanger.java:
##########
@@ -36,6 +39,8 @@ public class PortUnificationExchanger {
     private static final ErrorTypeAwareLogger log = LoggerFactory.getErrorTypeAwareLogger(PortUnificationExchanger.class);
     private static final ConcurrentMap<String, RemotingServer> servers = new ConcurrentHashMap<>();
 
+    private static volatile ConnectionManager connectionManager;

Review Comment:
   Just assign it directly in the property, because `PortUnificationExchanger` is a singleton, `MultiplexProtocolConnectionManager` is also a singleton
   



##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/PortUnificationExchanger.java:
##########
@@ -55,6 +60,22 @@ public static RemotingServer bind(URL url, ChannelHandler handler) {
         return servers.get(url.getAddress());
     }
 
+    public static AbstractConnectionClient connect(URL url, ChannelHandler handler) {
+        if (connectionManager == null) {
+            synchronized (PortUnificationExchanger.class) {
+                if (connectionManager == null) {
+                    connectionManager = url.getOrDefaultFrameworkModel().getExtensionLoader(ConnectionManager.class)
+                            .getExtension(MultiplexProtocolConnectionManager.NAME);
+                }
+            }
+        }
+
+        final AbstractConnectionClient connectionClient;
+        connectionClient = connectionManager.connect(url, handler);

Review Comment:
   This logic should be done in `NettyPortUnificationTransporter`
   



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2Protocol.java:
##########
@@ -173,8 +178,12 @@ public void configClientPipeline(URL url, ChannelPipeline pipeline, SslContext s
                     DEFAULT_MAX_HEADER_LIST_SIZE)))
             .frameLogger(CLIENT_LOGGER)
             .build();
-        final Http2MultiplexHandler handler = new Http2MultiplexHandler(
-            new TripleClientHandler(frameworkModel));
-        pipeline.addLast(codec, handler, new TripleTailHandler());
+        final Http2MultiplexHandler handler = new Http2MultiplexHandler(new SimpleChannelInboundHandler<Object>() {
+            @Override
+            protected void channelRead0(ChannelHandlerContext ctx, Object msg) {
+                logger.debug("triple client get msg from Http2MultiplexHandler: " + msg);

Review Comment:
   Remove unused log



##########
dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationTransporter.java:
##########
@@ -33,7 +33,7 @@ public AbstractPortUnificationServer bind(URL url, ChannelHandler handler) throw
     }
 
     @Override
-    public Client connect(URL url, ChannelHandler handler) throws RemotingException {
+    public AbstractConnectionClient connect(URL url, ChannelHandler handler) throws RemotingException {
         return null;
     }

Review Comment:
   Behavior should be consistent with dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationTransporter.java
   



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