-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-5932][CORE] Use consistent naming for size properties #5574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
d7a06b8
a99032f
e92caf7
8be2f83
733ec9f
dfec4da
0e1567c
15e8dea
270cfe3
cb0c6b7
8c884fa
1dc0444
db9a963
5390fd9
09ea450
747393a
a9f4fcf
851d691
475370a
0cdff35
b809a78
eba4de6
1fbd435
2d15681
ae7e9f6
afc9a38
7a6c847
5d29f90
928469e
35a7fa7
0f4443e
f15f209
f32bc01
69e2f20
54b78b4
c7803cd
fe286b4
d3d09b6
84a2581
8b43748
e428049
3dfae96
22413b1
9ee779c
852a407
fc85733
2ab886b
49a8720
11f6999
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…kryoserializer.buffer.mb and spark.reducer.maxMbInFlight
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,9 +49,9 @@ class KryoSerializer(conf: SparkConf) | |
| with Logging | ||
| with Serializable { | ||
|
|
||
| private val bufferSizeMb = conf.getDouble("spark.kryoserializer.buffer.mb", 0.064) | ||
| private val bufferSizeMb = conf.getSizeAsMb("spark.kryoserializer.buffer", "64k") | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note - Intentionally converted to 64k here instead of 0.064m since I assume this was the original intent and 0.064m does NOT equal 64k in general (mibi vs megabytes) |
||
| if (bufferSizeMb >= 2048) { | ||
| throw new IllegalArgumentException("spark.kryoserializer.buffer.mb must be less than " + | ||
| throw new IllegalArgumentException("spark.kryoserializer.buffer must be less than " + | ||
| s"2048 mb, got: + $bufferSizeMb mb.") | ||
| } | ||
| private val bufferSize = (bufferSizeMb * 1024 * 1024).toInt | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,7 +79,8 @@ private[hash] object BlockStoreShuffleFetcher extends Logging { | |
| blockManager, | ||
| blocksByAddress, | ||
| serializer, | ||
| SparkEnv.get.conf.getLong("spark.reducer.maxMbInFlight", 48) * 1024 * 1024) | ||
| // Note: we use getSizeAsMb to assume Mb when no suffix is provided | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| SparkEnv.get.conf.getSizeAsMb("spark.reducer.maxSizeInFlight", "48m") * 1024 * 1024) | ||
| val itr = blockFetcherItr.flatMap(unpackBlock) | ||
|
|
||
| val completionIter = CompletionIterator[T, Iterator[T]](itr, { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,46 +25,46 @@ public enum ByteUnit { | |
| }, | ||
|
|
||
| /** Kibibyte (KiB) = 1024 Byte */ | ||
| KB { | ||
| public long toBytes(long d) { return x(d, C_KB); } | ||
| KiB { | ||
| public long toBytes(long d) { return x(d, C_KiB); } | ||
|
|
||
| public long convert(long d, ByteUnit u) { return u.toKB(d); } | ||
| public long convert(long d, ByteUnit u) { return u.toKiB(d); } | ||
| }, | ||
|
|
||
| /** Mebibyte (MiB) = (1024^2) Byte */ | ||
| MB { | ||
| public long toBytes(long d) { return x(d, C_MB); } | ||
| MiB { | ||
| public long toBytes(long d) { return x(d, C_MiB); } | ||
|
|
||
| public long convert(long d, ByteUnit u) { return u.toMB(d); } | ||
| public long convert(long d, ByteUnit u) { return u.toMiB(d); } | ||
| }, | ||
|
|
||
| /** Gibibyte (GiB) = (1024^3) Byte */ | ||
| GB { | ||
| public long toBytes(long d) { return x(d, C_GB); | ||
| GiB { | ||
| public long toBytes(long d) { return x(d, C_GiB); | ||
| } | ||
|
|
||
| public long convert(long d, ByteUnit u) { return u.toGB(d); } | ||
| public long convert(long d, ByteUnit u) { return u.toGiB(d); } | ||
| }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: the new lines aren't super necessary |
||
|
|
||
| /** Tebibyte (TiB) = (1024^4) Byte */ | ||
| TB { | ||
| public long toBytes(long d) { return x(d, C_TB); } | ||
| TiB { | ||
| public long toBytes(long d) { return x(d, C_TiB); } | ||
|
|
||
| public long convert(long d, ByteUnit u) { return u.toTB(d); } | ||
| public long convert(long d, ByteUnit u) { return u.toTiB(d); } | ||
| }, | ||
|
|
||
| /** Pebibyte (PB) = (1024^5) Byte */ | ||
| PB { | ||
| public long toBytes(long d) { return x(d, C_PB); } | ||
| PiB { | ||
| public long toBytes(long d) { return x(d, C_PiB); } | ||
|
|
||
| public long convert(long d, ByteUnit u) { return u.toPB(d); } | ||
| public long convert(long d, ByteUnit u) { return u.toPiB(d); } | ||
| }; | ||
|
|
||
| static final long C_KB = 1024l; | ||
| static final long C_MB = (long) Math.pow(1024l, 2l); | ||
| static final long C_GB = (long) Math.pow(1024l, 3l); | ||
| static final long C_TB = (long) Math.pow(1024l, 4l); | ||
| static final long C_PB = (long) Math.pow(1024l, 5l); | ||
| static final long C_KiB = 1024l; | ||
| static final long C_MiB = (long) Math.pow(1024l, 2l); | ||
| static final long C_GiB = (long) Math.pow(1024l, 3l); | ||
| static final long C_TiB = (long) Math.pow(1024l, 4l); | ||
| static final long C_PiB = (long) Math.pow(1024l, 5l); | ||
|
|
||
| static final long MAX = Long.MAX_VALUE; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not used anywhere?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wanted to still keep these in case they were used down the line. Would you recommend getting rid of them? |
||
|
|
||
|
|
@@ -78,19 +78,18 @@ static long x(long d, long m) { | |
| if (d < -over) return Long.MIN_VALUE; | ||
|
||
| return d * m; | ||
| } | ||
|
|
||
| public long convert(long d, ByteUnit u) { throw new AbstractMethodError(); } | ||
|
||
|
|
||
| public long toBytes(long d) { throw new AbstractMethodError(); } | ||
| public long convert(long d, ByteUnit u) { throw new AbstractMethodError(); } | ||
|
|
||
| public long toKB(long d) { return toBytes(d) / C_KB; } | ||
| public long toKiB(long d) { return toBytes(d) / C_KiB; } | ||
|
|
||
| public long toMB(long d) { return toBytes(d) / C_MB; } | ||
| public long toMiB(long d) { return toBytes(d) / C_MiB; } | ||
|
|
||
| public long toGB(long d) { return toBytes(d) / C_GB; } | ||
| public long toGiB(long d) { return toBytes(d) / C_GiB; } | ||
|
|
||
| public long toTB(long d) { return toBytes(d) / C_TB; } | ||
| public long toTiB(long d) { return toBytes(d) / C_TiB; } | ||
|
|
||
| public long toPB(long d) { return toBytes(d) / C_PB; } | ||
|
|
||
|
|
||
| public long toPiB(long d) { return toBytes(d) / C_PiB; } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have a translation function for this, no? Like
spark.yarn.applicationMaster.waitTriesbelow.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automatic translation would fail with a NumberFormatException though if they try to pass in a correctly formatted new value for the old config (e.g. 64k) and there's nowhere to catch that exception in this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you added the alternate config, you can remove this entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to still provide this message to clarify that fractional values are no longer allowed.