You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "luoluoyuyu (via GitHub)" <gi...@apache.org> on 2024/07/12 08:02:34 UTC

[PR] Pipe: Fix client call to returnSelf function early [iotdb]

luoluoyuyu opened a new pull request, #12913:
URL: https://github.com/apache/iotdb/pull/12913

   As title.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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


Re: [PR] Pipe: Fix client call to returnSelf function early [iotdb]

Posted by "Caideyipi (via GitHub)" <gi...@apache.org>.
Caideyipi commented on code in PR #12913:
URL: https://github.com/apache/iotdb/pull/12913#discussion_r1675540203


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/client/IoTDBDataNodeAsyncClientManager.java:
##########
@@ -233,7 +234,7 @@ public void onError(Exception e) {
           callback);
       waitHandshakeFinished(isHandshakeFinished);
     }
-
+    client.setShouldReturnSelf(true);
     if (exception.get() != null) {
       throw new PipeConnectionException("Failed to handshake.", exception.get());

Review Comment:
   If there are exceptions, maybe we should return the client, or else it's leaked and cannot be borrowed again...



-- 
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: reviews-unsubscribe@iotdb.apache.org

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


Re: [PR] Pipe: Fix async client returning self too early when downgrade from handshake v2 to v1 [iotdb]

Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu merged PR #12913:
URL: https://github.com/apache/iotdb/pull/12913


-- 
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: reviews-unsubscribe@iotdb.apache.org

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


Re: [PR] Pipe: Fix client call to returnSelf function early [iotdb]

Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu commented on code in PR #12913:
URL: https://github.com/apache/iotdb/pull/12913#discussion_r1677273186


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/client/IoTDBDataNodeAsyncClientManager.java:
##########
@@ -199,43 +199,48 @@ public void onError(Exception e) {
           }
         };
 
-    // Try to handshake by PipeTransferHandshakeV2Req.
-    final HashMap<String, String> params = new HashMap<>();
-    params.put(
-        PipeTransferHandshakeConstant.HANDSHAKE_KEY_CLUSTER_ID,
-        IoTDBDescriptor.getInstance().getConfig().getClusterId());
-    params.put(
-        PipeTransferHandshakeConstant.HANDSHAKE_KEY_TIME_PRECISION,
-        CommonDescriptor.getInstance().getConfig().getTimestampPrecision());
-
-    client.setTimeoutDynamically(PipeConfig.getInstance().getPipeConnectorHandshakeTimeoutMs());
-    client.pipeTransfer(PipeTransferDataNodeHandshakeV2Req.toTPipeTransferReq(params), callback);
-    waitHandshakeFinished(isHandshakeFinished);
-
-    // Retry to handshake by PipeTransferHandshakeV1Req.
-    if (resp.get() != null
-        && resp.get().getStatus().getCode() == TSStatusCode.PIPE_TYPE_ERROR.getStatusCode()) {
-      LOGGER.info(
-          "Handshake error by PipeTransferHandshakeV2Req with receiver {}:{} "
-              + "retry to handshake by PipeTransferHandshakeV1Req.",
-          targetNodeUrl.getIp(),
-          targetNodeUrl.getPort());
-
-      supportModsIfIsDataNodeReceiver = false;
-      isHandshakeFinished.set(false);
-      resp.set(null);
-      exception.set(null);
+    try {
+      client.setShouldReturnSelf(false);
+      // Try to handshake by PipeTransferHandshakeV2Req.
+      final HashMap<String, String> params = new HashMap<>();
+      params.put(
+          PipeTransferHandshakeConstant.HANDSHAKE_KEY_CLUSTER_ID,
+          IoTDBDescriptor.getInstance().getConfig().getClusterId());
+      params.put(
+          PipeTransferHandshakeConstant.HANDSHAKE_KEY_TIME_PRECISION,
+          CommonDescriptor.getInstance().getConfig().getTimestampPrecision());
 
       client.setTimeoutDynamically(PipeConfig.getInstance().getPipeConnectorHandshakeTimeoutMs());
-      client.pipeTransfer(
-          PipeTransferDataNodeHandshakeV1Req.toTPipeTransferReq(
-              CommonDescriptor.getInstance().getConfig().getTimestampPrecision()),
-          callback);
+      client.pipeTransfer(PipeTransferDataNodeHandshakeV2Req.toTPipeTransferReq(params), callback);
       waitHandshakeFinished(isHandshakeFinished);
-    }
 
-    if (exception.get() != null) {
-      throw new PipeConnectionException("Failed to handshake.", exception.get());
+      // Retry to handshake by PipeTransferHandshakeV1Req.
+      if (resp.get() != null
+          && resp.get().getStatus().getCode() == TSStatusCode.PIPE_TYPE_ERROR.getStatusCode()) {
+        LOGGER.info(
+            "Handshake error by PipeTransferHandshakeV2Req with receiver {}:{} "
+                + "retry to handshake by PipeTransferHandshakeV1Req.",
+            targetNodeUrl.getIp(),
+            targetNodeUrl.getPort());
+
+        supportModsIfIsDataNodeReceiver = false;
+        isHandshakeFinished.set(false);
+        resp.set(null);
+        exception.set(null);
+
+        client.setTimeoutDynamically(PipeConfig.getInstance().getPipeConnectorHandshakeTimeoutMs());
+        client.pipeTransfer(
+            PipeTransferDataNodeHandshakeV1Req.toTPipeTransferReq(
+                CommonDescriptor.getInstance().getConfig().getTimestampPrecision()),
+            callback);
+        waitHandshakeFinished(isHandshakeFinished);
+      }
+      if (exception.get() != null) {

Review Comment:
   ```suggestion
         }
         
         if (exception.get() != null) {
   ```



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/client/IoTDBDataNodeAsyncClientManager.java:
##########
@@ -199,43 +199,48 @@ public void onError(Exception e) {
           }
         };
 
-    // Try to handshake by PipeTransferHandshakeV2Req.
-    final HashMap<String, String> params = new HashMap<>();
-    params.put(
-        PipeTransferHandshakeConstant.HANDSHAKE_KEY_CLUSTER_ID,
-        IoTDBDescriptor.getInstance().getConfig().getClusterId());
-    params.put(
-        PipeTransferHandshakeConstant.HANDSHAKE_KEY_TIME_PRECISION,
-        CommonDescriptor.getInstance().getConfig().getTimestampPrecision());
-
-    client.setTimeoutDynamically(PipeConfig.getInstance().getPipeConnectorHandshakeTimeoutMs());
-    client.pipeTransfer(PipeTransferDataNodeHandshakeV2Req.toTPipeTransferReq(params), callback);
-    waitHandshakeFinished(isHandshakeFinished);
-
-    // Retry to handshake by PipeTransferHandshakeV1Req.
-    if (resp.get() != null
-        && resp.get().getStatus().getCode() == TSStatusCode.PIPE_TYPE_ERROR.getStatusCode()) {
-      LOGGER.info(
-          "Handshake error by PipeTransferHandshakeV2Req with receiver {}:{} "
-              + "retry to handshake by PipeTransferHandshakeV1Req.",
-          targetNodeUrl.getIp(),
-          targetNodeUrl.getPort());
-
-      supportModsIfIsDataNodeReceiver = false;
-      isHandshakeFinished.set(false);
-      resp.set(null);
-      exception.set(null);
+    try {
+      client.setShouldReturnSelf(false);
+      // Try to handshake by PipeTransferHandshakeV2Req.
+      final HashMap<String, String> params = new HashMap<>();

Review Comment:
   ```suggestion
         client.setShouldReturnSelf(false);
         
         // Try to handshake by PipeTransferHandshakeV2Req.
         final HashMap<String, String> params = new HashMap<>();
   ```



-- 
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: reviews-unsubscribe@iotdb.apache.org

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