Victsm commented on a change in pull request #25907: [SPARK-29206][SHUFFLE] 
Make number of shuffle server threads a multiple of number of chunk fetch 
handler threads.
URL: https://github.com/apache/spark/pull/25907#discussion_r328299644
 
 

 ##########
 File path: 
common/network-common/src/main/java/org/apache/spark/network/util/TransportConf.java
 ##########
 @@ -111,8 +111,30 @@ public int numConnectionsPerPeer() {
   /** Requested maximum length of the queue of incoming connections. Default 
is 64. */
   public int backLog() { return conf.getInt(SPARK_NETWORK_IO_BACKLOG_KEY, 64); 
}
 
-  /** Number of threads used in the server thread pool. Default to 0, which is 
2x#cores. */
-  public int serverThreads() { return 
conf.getInt(SPARK_NETWORK_IO_SERVERTHREADS_KEY, 0); }
+  /**
+   * The configured ratio between number of server threads and number of chunk 
fetch handler
+   * threads. Default to 1, which sets the size of both thread pools to be 
equal. Number of
+   * server threads needs to be a multiple of this ratio. See SPARK-29206.
+   */
+  private int getChunkFetchHandlerThreadsRatio() {
+    return conf.getInt("spark.shuffle.server.chunkFetchHandlerThreadsRatio", 
1);
+  }
+
+  /**
+   * Number of threads used in the server thread pool. Default to 0, which is 
2x#cores.
+   * If spark.shuffle.server.chunkFetchHandlerThreadsRatio is configured, and 
the Netty server
+   * is for shuffle, then the actual # of server threads will round up to the 
nearest int that
+   * is a multiple of the configured ratio.
+   */
+  public int serverThreads() {
+    int configuredServerThreads = 
conf.getInt(SPARK_NETWORK_IO_SERVERTHREADS_KEY, 0);
+    if (this.getModuleName().equalsIgnoreCase("shuffle")) {
+      int chunkFetchHandlerThreadsRatio = getChunkFetchHandlerThreadsRatio();
+      return (int) Math.ceil(configuredServerThreads / 
(chunkFetchHandlerThreadsRatio * 1.0));
 
 Review comment:
   If we leave the config like before, user could configure serverThreads as 9, 
and the percent as 50%. It would then use 9 server threads and 5 chunk fetch 
handler threads. Since 9 is not a multiple of 5, with this config we might 
experience RPC message timeout issue again.
   
   With the ratio config, user could configure serverThreads as 9 and ratio as 
2. Then, with this patch, we would configure 10 server threads and 5 chunk 
fetch handler threads. This would guarantee to avoid the issue we saw.
   
   It still works with the default case as serverThreads() will still return 0 
here. The # chunk fetch handler threads will then be configured as 2 * # cores, 
same as the number of server threads. This is the same as the current behavior.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to