[GitHub] [spark] attilapiros commented on a change in pull request #23695: [SPARK-26780][CORE]Improve shuffle read using ReadAheadInputStream

[GitBox <gi...@apache.org>]

attilapiros commented on a change in pull request #23695: [SPARK-26780][CORE]Improve shuffle read using ReadAheadInputStream 
URL: https://github.com/apache/spark/pull/23695#discussion_r262887733
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala
 ##########
 @@ -474,6 +475,12 @@ final class ShuffleBlockFetcherIterator(
               // TODO: manage the memory used here, and spill it into disk in case of OOM.
               Utils.copyStream(input, out, closeStreams = true)
               input = out.toChunkedByteBuffer.toInputStream(dispose = true)
+            } else {
+              val readAheadEnabled = SparkEnv.get != null &&
+                SparkEnv.get.conf.get(config.SHUFFLE_READ_AHEAD)
+              if (readAheadEnabled) {
+                input = new ReadAheadInputStream(input, 1024 * 1024)
 
 Review comment:
   I am just wondering why and how 1MB is chosen here. I know in Spark we have already too many config values but what is your opinion using only one config  `spark.shuffle.read.ahead.buffer.size` which could be `bytesConf` and when it is 0 then wrapping into `ReadAheadInputStream` is skipped (like in case of disabled now). 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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