[GitHub] [spark] MaxGekk commented on a diff in pull request #41140: [SPARK-38469][CORE] Use error class in org.apache.spark.network

["MaxGekk (via GitHub)" <gi...@apache.org>]

MaxGekk commented on code in PR #41140:
URL: https://github.com/apache/spark/pull/41140#discussion_r1191221294


##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -93,8 +94,8 @@ class NettyBlockRpcServer(
           } else {
             val startAndEndId = fetchShuffleBlocks.reduceIds(index)
             if (startAndEndId.length != 2) {
-              throw new IllegalStateException(s"Invalid shuffle fetch request when batch mode " +
-                s"is enabled: $fetchShuffleBlocks")
+              throw SparkException.internalError(s"Invalid shuffle fetch request when batch mode " +
+                s"is enabled: $fetchShuffleBlocks", category = "INTERNAL_ERROR_NETWORK")

Review Comment:
   Just `NETWORK`, I think
   ```suggestion
                   s"is enabled: $fetchShuffleBlocks", category = "NETWORK")
   ```



##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -93,8 +94,8 @@ class NettyBlockRpcServer(
           } else {
             val startAndEndId = fetchShuffleBlocks.reduceIds(index)
             if (startAndEndId.length != 2) {
-              throw new IllegalStateException(s"Invalid shuffle fetch request when batch mode " +
-                s"is enabled: $fetchShuffleBlocks")
+              throw SparkException.internalError(s"Invalid shuffle fetch request when batch mode " +

Review Comment:
   ```suggestion
                 throw SparkException.internalError("Invalid shuffle fetch request when batch mode " +
   ```



##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -137,13 +140,15 @@ class NettyBlockRpcServer(
           val errorMsg = "Invalid GetLocalDirsForExecutors request: " +
             s"${if (isIncorrectAppId) s"incorrect application id: ${getLocalDirs.appId};"}" +
             s"${if (execNum != 1) s"incorrect executor number: $execNum (expected 1);"}"
-          responseContext.onFailure(new IllegalStateException(errorMsg))
+          responseContext.onFailure(
+            SparkException.internalError(errorMsg, "INTERNAL_ERROR_NETWORK"))
         } else {
           val expectedExecId = blockManager.asInstanceOf[BlockManager].executorId
           val actualExecId = getLocalDirs.execIds.head
           if (actualExecId != expectedExecId) {
-            responseContext.onFailure(new IllegalStateException(
-              s"Invalid executor id: $actualExecId, expected $expectedExecId."))
+            responseContext.onFailure(SparkException.internalError(
+              s"Invalid executor id: $actualExecId, expected $expectedExecId.",
+              "INTERNAL_ERROR_NETWORK"))

Review Comment:
   ```suggestion
                 category = "NETWORK"))
   ```



##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -137,13 +140,15 @@ class NettyBlockRpcServer(
           val errorMsg = "Invalid GetLocalDirsForExecutors request: " +
             s"${if (isIncorrectAppId) s"incorrect application id: ${getLocalDirs.appId};"}" +
             s"${if (execNum != 1) s"incorrect executor number: $execNum (expected 1);"}"
-          responseContext.onFailure(new IllegalStateException(errorMsg))
+          responseContext.onFailure(
+            SparkException.internalError(errorMsg, "INTERNAL_ERROR_NETWORK"))

Review Comment:
   ```suggestion
               SparkException.internalError(errorMsg, category = "NETWORK"))
   ```



##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -125,8 +126,10 @@ class NettyBlockRpcServer(
         if (blockStored) {
           responseContext.onSuccess(ByteBuffer.allocate(0))
         } else {
-          val exception = new Exception(s"Upload block for $blockId failed. This mostly happens " +
-            s"when there is not sufficient space available to store the block.")
+          val exception = SparkException.internalError(
+            s"Upload block for $blockId failed. This mostly happens " +
+            s"when there is not sufficient space available to store the block.",

Review Comment:
   s is not needed here
   ```suggestion
               "when there is not sufficient space available to store the block.",
   ```



##########
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala:
##########
@@ -125,8 +126,10 @@ class NettyBlockRpcServer(
         if (blockStored) {
           responseContext.onSuccess(ByteBuffer.allocate(0))
         } else {
-          val exception = new Exception(s"Upload block for $blockId failed. This mostly happens " +
-            s"when there is not sufficient space available to store the block.")
+          val exception = SparkException.internalError(
+            s"Upload block for $blockId failed. This mostly happens " +
+            s"when there is not sufficient space available to store the block.",
+            category = "INTERNAL_ERROR_NETWORK")

Review Comment:
   ```suggestion
               category = "NETWORK")
   ```



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