You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "grundprinzip (via GitHub)" <gi...@apache.org> on 2023/03/28 19:04:36 UTC

[GitHub] [spark] grundprinzip commented on a diff in pull request #40575: [SPARK-42945][CONNECT] Support PYSPARK_JVM_STACKTRACE_ENABLED in Spark Connect

grundprinzip commented on code in PR #40575:
URL: https://github.com/apache/spark/pull/40575#discussion_r1151045107


##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala:
##########
@@ -73,18 +74,26 @@ class SparkConnectService(debug: Boolean)
     classes.toSeq
   }
 
-  private def buildStatusFromThrowable(st: Throwable): RPCStatus = {
+  private def buildStatusFromThrowable(
+      st: Throwable,
+      stackTraceEnabled: Boolean = false): RPCStatus = {
+    val errorInfo = ErrorInfo
+      .newBuilder()
+      .setReason(st.getClass.getName)
+      .setDomain("org.apache.spark")
+      .putMetadata("classes", compact(render(allClasses(st.getClass).map(_.getName))))
+
+    val withStackTrace = if (stackTraceEnabled) {
+      val stackTrace = ExceptionUtils.getStackTrace(st)
+      errorInfo.putMetadata("stackTrace", StringUtils.abbreviate(stackTrace, 4096))
+    } else {
+      errorInfo
+    }
+
     RPCStatus
       .newBuilder()
       .setCode(RPCCode.INTERNAL_VALUE)
-      .addDetails(
-        ProtoAny.pack(
-          ErrorInfo
-            .newBuilder()
-            .setReason(st.getClass.getName)
-            .setDomain("org.apache.spark")
-            .putMetadata("classes", compact(render(allClasses(st.getClass).map(_.getName))))
-            .build()))
+      .addDetails(ProtoAny.pack(withStackTrace.build()))

Review Comment:
   Adding the stack trace can break the result handling. Depending on the intermediate server the stack trace might be too big for single HTTP2 frames. We internally increased this to 128k but this is not the default.
   
   This might make it hard to debug for an intermediate party when they deploy a proxy in front of Spark Connect.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org