[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/22754 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r228738630 --- Diff: core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeSorterSpillWriter.java --- @@ -42,7 +42,9 @@ private final SparkConf conf = new SparkConf(); - /** The buffer size to use when writing the sorted records to an on-disk file */ + /** The buffer size to use when writing the sorted records to an on-disk file, and --- End diff -- nit: For a multiple-line comment, a starting line `/**` does not have a text. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r228601036 --- Diff: core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeSorterSpillWriter.java --- @@ -62,6 +62,8 @@ public UnsafeSorterSpillWriter( int fileBufferSize, ShuffleWriteMetrics writeMetrics, int numRecordsToWrite) throws IOException { +// Space used by prefix + len + recordLength is more than 4 + 8 bytes +assert (diskWriteBufferSize > 12); --- End diff -- Where is the best place of this comment? I am neutral on this. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user 10110346 commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r228009755 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -495,8 +495,8 @@ package object config { ConfigBuilder("spark.shuffle.spill.diskWriteBufferSize") .doc("The buffer size, in bytes, to use when writing the sorted records to an on-disk file.") .bytesConf(ByteUnit.BYTE) - .checkValue(v => v > 0 && v <= Int.MaxValue, -s"The buffer size must be greater than 0 and less than ${Int.MaxValue}.") + .checkValue(v => v > 12 && v <= Int.MaxValue, +s"The buffer size must be greater than 12 and less than ${Int.MaxValue}.") --- End diff -- Yea, I think this value is better --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r227780781 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -495,8 +495,8 @@ package object config { ConfigBuilder("spark.shuffle.spill.diskWriteBufferSize") .doc("The buffer size, in bytes, to use when writing the sorted records to an on-disk file.") .bytesConf(ByteUnit.BYTE) - .checkValue(v => v > 0 && v <= Int.MaxValue, -s"The buffer size must be greater than 0 and less than ${Int.MaxValue}.") + .checkValue(v => v > 12 && v <= Int.MaxValue, +s"The buffer size must be greater than 12 and less than ${Int.MaxValue}.") --- End diff -- How about using [this value](https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/array/ByteArrayMethods.java#L52)? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r227729436 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -495,8 +495,8 @@ package object config { ConfigBuilder("spark.shuffle.spill.diskWriteBufferSize") .doc("The buffer size, in bytes, to use when writing the sorted records to an on-disk file.") .bytesConf(ByteUnit.BYTE) - .checkValue(v => v > 0 && v <= Int.MaxValue, -s"The buffer size must be greater than 0 and less than ${Int.MaxValue}.") + .checkValue(v => v > 12 && v <= Int.MaxValue, +s"The buffer size must be greater than 12 and less than ${Int.MaxValue}.") --- End diff -- Thanks, I image this is due to a recent issue in Github. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user 10110346 commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r227392626 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -495,8 +495,8 @@ package object config { ConfigBuilder("spark.shuffle.spill.diskWriteBufferSize") .doc("The buffer size, in bytes, to use when writing the sorted records to an on-disk file.") .bytesConf(ByteUnit.BYTE) - .checkValue(v => v > 0 && v <= Int.MaxValue, -s"The buffer size must be greater than 0 and less than ${Int.MaxValue}.") + .checkValue(v => v > 12 && v <= Int.MaxValue, +s"The buffer size must be greater than 12 and less than ${Int.MaxValue}.") --- End diff -- So strange, l have handled it, but we can not see the change here --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22754: [SPARK-25776][CORE]The disk write buffer size mus...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22754#discussion_r227363331 --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala --- @@ -495,8 +495,8 @@ package object config { ConfigBuilder("spark.shuffle.spill.diskWriteBufferSize") .doc("The buffer size, in bytes, to use when writing the sorted records to an on-disk file.") .bytesConf(ByteUnit.BYTE) - .checkValue(v => v > 0 && v <= Int.MaxValue, -s"The buffer size must be greater than 0 and less than ${Int.MaxValue}.") + .checkValue(v => v > 12 && v <= Int.MaxValue, +s"The buffer size must be greater than 12 and less than ${Int.MaxValue}.") --- End diff -- Sorry for bothering you. Can we handle this in this PR, too? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org