You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/03/14 02:50:04 UTC

[GitHub] [incubator-kyuubi] ulysses-you commented on a change in pull request #2122: [KYUUBI #2102] Support to retry the internal thrift RPC call and fast fail on transport closed or instability

ulysses-you commented on a change in pull request #2122:
URL: https://github.com/apache/incubator-kyuubi/pull/2122#discussion_r825558711



##########
File path: kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala
##########
@@ -55,6 +57,47 @@ class KyuubiSyncThriftClient private (protocol: TProtocol)
     } finally lock.unlock()
   }
 
+  private def withRetryingRpc[T](block: => T, rpcName: String): T = {
+    var retryCount = 0
+
+    def shouldRetry(e: Throwable): Boolean = {
+      e.isInstanceOf[TException] && retryCount < maxRetries && !isClosed() && isConnectionValid()
+    }
+
+    def isConnectionValid(): Boolean = {
+      var isValid = false
+      val tGetInfoReq = new TGetInfoReq()
+      tGetInfoReq.setSessionHandle(_remoteSessionHandle)
+      tGetInfoReq.setInfoType(TGetInfoType.CLI_DBMS_VER)
+      try {
+        withLockAcquired(GetInfo(tGetInfoReq).getInfoValue.getStringValue)
+        isValid = true
+      } catch {
+        case e: Throwable =>
+          error(s"The internal connection is not valid, mark the thrift client as closed", e)
+          setClosed()
+      }
+      isValid
+    }
+
+    try {
+      withLockAcquired(block)

Review comment:
       It seems the lock should be high level. It shoud be atomical even if we retry one operation with several rpcs. That said  the lock is working between operation and operation, not rpc and rpc.




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

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



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