Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
59bf9e1
[SPARK-5931] Updated Utils and JavaUtils classes to add helper method…
Mar 27, 2015
404f8c3
Updated usage of spark.core.connection.ack.wait.timeout
Mar 27, 2015
7db6d2a
Updated usage of spark.akka.timeout
Mar 27, 2015
4933fda
Updated usage of spark.storage.blockManagerSlaveTimeout
Mar 27, 2015
c9f5cad
Updated spark.shuffle.io.retryWait
Mar 27, 2015
21ef3dd
updated spark.shuffle.sasl.timeout
Mar 27, 2015
064ebd6
Updated usage of spark.cleaner.ttl
Mar 27, 2015
7320c87
updated spark.akka.heartbeat.interval
Mar 27, 2015
272c215
Updated spark.locality.wait
Mar 27, 2015
3352d34
Updated spark.scheduler.maxRegisteredResourcesWaitingTime
Mar 27, 2015
3f1cfc8
Updated spark.scheduler.revive.interval
Mar 27, 2015
6d1518e
Upated spark.speculation.interval
Mar 27, 2015
2fcc91c
Updated spark.dynamicAllocation.executorIdleTimeout
Mar 27, 2015
5181597
Updated spark.dynamicAllocation.schedulerBacklogTimeout
Mar 27, 2015
c6a0095
Updated spark.core.connection.auth.wait.timeout
Mar 27, 2015
cde9bff
Updated spark.streaming.blockInterval
Mar 27, 2015
42477aa
Updated configuration doc with note on specifying time properties
Mar 27, 2015
9a29d8d
Fixed misuse of time in streaming context test
Mar 27, 2015
34f87c2
Update Utils.scala
ilganeli Mar 28, 2015
8f741e1
Update JavaUtils.java
ilganeli Mar 28, 2015
9e2547c
Reverting doc changes
Mar 30, 2015
499bdf0
Merge branch 'SPARK-5931' of github.com:ilganeli/spark into SPARK-5931
Mar 30, 2015
5232a36
[SPARK-5931] Changed default behavior of time string conversion.
Mar 30, 2015
3a12dd8
Updated host revceiver
Mar 30, 2015
68f4e93
Updated more files to clean up usage of default time strings
Mar 30, 2015
70ac213
Fixed remaining usages to be consistent. Updated Java-side time conve…
Mar 30, 2015
647b5ac
Udpated time conversion to use map iterator instead of if fall through
Mar 31, 2015
1c0c07c
Updated Java code to add day, minutes, and hours
Mar 31, 2015
8613631
Whitespace
Mar 31, 2015
bac9edf
More whitespace
Mar 31, 2015
1858197
Fixed bug where all time was being converted to us instead of the app…
Mar 31, 2015
3b126e1
Fixed conversion to US from seconds
Mar 31, 2015
39164f9
[SPARK-5931] Updated Java conversion to be similar to scala conversio…
Mar 31, 2015
b2fc965
replaced get or default since it's not present in this version of java
Mar 31, 2015
dd0a680
Updated scala code to call into java
Mar 31, 2015
bf779b0
Special handling of overlapping usffixes for java
Apr 1, 2015
76cfa27
[SPARK-5931] Minor nit fixes'
Apr 1, 2015
5193d5f
Resolved merge conflicts
Apr 6, 2015
6387772
Updated suffix handling to handle overlap of units more gracefully
Apr 6, 2015
19c31af
Added cleaner computation of time conversions in tests
Apr 6, 2015
ff40bfe
Updated tests to fix small bugs
Apr 6, 2015
28187bf
Convert straight to seconds
Apr 6, 2015
1465390
Nit
Apr 6, 2015
cbf41db
Got rid of thrown exceptions
Apr 7, 2015
d4efd26
Added time conversion for yarn.scheduler.heartbeat.interval-ms
Apr 8, 2015
4e48679
Fixed priority order and mixed up conversions in a couple spots
Apr 8, 2015
1a1122c
Formatting fixes and added m for use as minute formatter
Apr 8, 2015
cbd2ca6
Formatting error
Apr 8, 2015
6f651a8
Now using regexes to simplify code in parseTimeString. Introduces get…
Apr 8, 2015
7d19cdd
Added fix for possible NPE
Apr 8, 2015
dc7bd08
Fixed error in exception handling
Apr 8, 2015
69fedcc
Added test for zero
Apr 8, 2015
8927e66
Fixed handling of -1
Apr 9, 2015
642a06d
Fixed logic for invalid suffixes and addid matching test
Apr 9, 2015
25d3f52
Minor nit fixes
Apr 11, 2015
bc04e05
Minor fixes and doc updates
Apr 13, 2015
951ca2d
Made the most recent round of changes
Apr 13, 2015
f5fafcd
Doc updates
Apr 13, 2015
de3bff9
Fixing style errors
Apr 13, 2015
4526c81
Update configuration.md
Apr 13, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updated usage of spark.core.connection.ack.wait.timeout
  • Loading branch information
Ilya Ganelin committed Mar 27, 2015
commit 404f8c3762d7e23fdf3ef8d158d68f5b5aa38360
8 changes: 4 additions & 4 deletions core/src/main/scala/org/apache/spark/executor/Executor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -442,14 +442,14 @@ private[spark] class Executor(
* This thread stops running when the executor is stopped.
*/
private def startDriverHeartbeater(): Unit = {
val interval = conf.getInt("spark.executor.heartbeatInterval", 10000)
val intervalMs = Utils.timeStringToMs(conf.get("spark.executor.heartbeatInterval", "10s"))
val thread = new Thread() {
override def run() {
// Sleep a random interval so the heartbeats don't end up in sync
Thread.sleep(interval + (math.random * interval).asInstanceOf[Int])
// Sleep a random intervalMs so the heartbeats don't end up in sync
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the "interval" here was referring to the variable, so could you please revert this :)

Thread.sleep(intervalMs + (math.random * intervalMs).asInstanceOf[Int])
while (!isStopped) {
reportHeartBeat()
Thread.sleep(interval)
Thread.sleep(intervalMs)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/scala/org/apache/spark/util/Utils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,9 @@ private[spark] object Utils extends Logging {
}
Utils.setupSecureURLConnection(uc, securityMgr)

val timeout = conf.getInt("spark.files.fetchTimeout", 60) * 1000
uc.setConnectTimeout(timeout)
uc.setReadTimeout(timeout)
val timeoutMs = Utils.timeStringToMs(conf.get("spark.files.fetchTimeout", "60s")).toInt
uc.setConnectTimeout(timeoutMs)
uc.setReadTimeout(timeoutMs)
uc.connect()
val in = uc.getInputStream()
downloadFile(url, in, targetFile, fileOverwrite)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ class ConnectionManagerSuite extends FunSuite {
val clientConf = new SparkConf
clientConf.set("spark.authenticate", "false")
val ackTimeout = 30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could consider renaming this acktimeoutS too, but not that important either way.

clientConf.set("spark.core.connection.ack.wait.timeout", s"${ackTimeout}")
clientConf.set("spark.core.connection.ack.wait.timeout", s"${ackTimeout}s")

val clientSecurityManager = new SecurityManager(clientConf)
val manager = new ConnectionManager(0, clientConf, clientSecurityManager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class BlockManagerReplicationSuite extends FunSuite with Matchers with BeforeAnd
conf.set("spark.storage.unrollMemoryThreshold", "512")

// to make a replication attempt to inactive store fail fast
conf.set("spark.core.connection.ack.wait.timeout", "1")
conf.set("spark.core.connection.ack.wait.timeout", "1s")
// to make cached peers refresh frequently
conf.set("spark.storage.cachedPeersTtl", "10")

Expand Down
58 changes: 29 additions & 29 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ of the most common options to set are:
<td>
Amount of memory to use for the driver process, i.e. where SparkContext is initialized.
(e.g. <code>512m</code>, <code>2g</code>).

<br /><em>Note:</em> In client mode, this config must not be set through the <code>SparkConf</code>
directly in your application, because the driver JVM has already started at that point.
Instead, please set this through the <code>--driver-memory</code> command line option
Expand Down Expand Up @@ -188,7 +188,7 @@ Apart from these, the following properties are also available, and may be useful

<br /><em>Note:</em> In client mode, this config must not be set through the <code>SparkConf</code>
directly in your application, because the driver JVM has already started at that point.
Instead, please set this through the <code>--driver-class-path</code> command line option or in
Instead, please set this through the <code>--driver-class-path</code> command line option or in
your default properties file.</td>
</td>
</tr>
Expand All @@ -197,10 +197,10 @@ Apart from these, the following properties are also available, and may be useful
<td>(none)</td>
<td>
A string of extra JVM options to pass to the driver. For instance, GC settings or other logging.

<br /><em>Note:</em> In client mode, this config must not be set through the <code>SparkConf</code>
directly in your application, because the driver JVM has already started at that point.
Instead, please set this through the <code>--driver-java-options</code> command line option or in
Instead, please set this through the <code>--driver-java-options</code> command line option or in
your default properties file.</td>
</td>
</tr>
Expand All @@ -209,10 +209,10 @@ Apart from these, the following properties are also available, and may be useful
<td>(none)</td>
<td>
Set a special library path to use when launching the driver JVM.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal but I see a fair bit of white-space-only change in the diff. It makes it a little harder to see what's changed. I don't know how much it's possible or worth fixing by hand but might check your IDE settings and not strip trailing spaces.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srowen I gave this some thought. I think we can both support the new interface and maintain compatibility.

Put simply, throughout the code I use Utils.timeStringToMs or Utils.timeStringToS for all specified constants as I do now. The only thing thing that needs to change is the default behavior for these. E.g. if the parameter is read in the code with timeStringToMs then if the format is unspecified (600 vs 600 ms) the expected unit will be inferred from the function used. Otherwise, whichever unit is provided is used.

This will allow us to introduce the new interface, update all usage of constants throughout the code to clarify units, and to support old usage. It will also cut down on the number of code changes since documentation can be left as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sean - to follow up on the white-space comment. The reason that there's white space changes is that there's a lot of trailing white space in these files which my IDE is stripping. I could disable white-space stripping but in this case, I think that this is a fix worth making since it shouldn't be there in the first place.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would turn it off. The whitespace itself doesn't matter at all functionally in most contexts. But it can create more noise in the diff. More important it can create spurious merge conflicts. An instance here or there doesn't matter much but I actively try to not make whitespace changes for this reason.

<br /><em>Note:</em> In client mode, this config must not be set through the <code>SparkConf</code>
directly in your application, because the driver JVM has already started at that point.
Instead, please set this through the <code>--driver-library-path</code> command line option or in
Instead, please set this through the <code>--driver-library-path</code> command line option or in
your default properties file.</td>
</td>
</tr>
Expand All @@ -223,26 +223,26 @@ Apart from these, the following properties are also available, and may be useful
(Experimental) Whether to give user-added jars precedence over Spark's own jars when loading
classes in the the driver. This feature can be used to mitigate conflicts between Spark's
dependencies and user dependencies. It is currently an experimental feature.

This is used in cluster mode only.
</td>
</tr>
<tr>
<td><code>spark.executor.extraClassPath</code></td>
<td>(none)</td>
<td>
Extra classpath entries to append to the classpath of executors. This exists primarily for
backwards-compatibility with older versions of Spark. Users typically should not need to set
Extra classpath entries to append to the classpath of executors. This exists primarily for
backwards-compatibility with older versions of Spark. Users typically should not need to set
this option.
</td>
</tr>
<tr>
<td><code>spark.executor.extraJavaOptions</code></td>
<td>(none)</td>
<td>
A string of extra JVM options to pass to executors. For instance, GC settings or other logging.
Note that it is illegal to set Spark properties or heap size settings with this option. Spark
properties should be set using a SparkConf object or the spark-defaults.conf file used with the
A string of extra JVM options to pass to executors. For instance, GC settings or other logging.
Note that it is illegal to set Spark properties or heap size settings with this option. Spark
properties should be set using a SparkConf object or the spark-defaults.conf file used with the
spark-submit script. Heap size settings can be set with spark.executor.memory.
</td>
</tr>
Expand Down Expand Up @@ -732,17 +732,17 @@ Apart from these, the following properties are also available, and may be useful
</tr>
<tr>
<td><code>spark.executor.heartbeatInterval</code></td>
<td>10000</td>
<td>Interval (milliseconds) between each executor's heartbeats to the driver. Heartbeats let
<td>10s</td>
<td>Interval between each executor's heartbeats to the driver. Heartbeats let
the driver know that the executor is still alive and update it with metrics for in-progress
tasks.</td>
</tr>
<tr>
<td><code>spark.files.fetchTimeout</code></td>
<td>60</td>
<td>60s</td>
<td>
Communication timeout to use when fetching files added through SparkContext.addFile() from
the driver, in seconds.
the driver.
</td>
</tr>
<tr>
Expand Down Expand Up @@ -855,14 +855,14 @@ Apart from these, the following properties are also available, and may be useful
<td><code>spark.akka.heartbeat.interval</code></td>
<td>1000</td>
<td>
This is set to a larger value to disable the transport failure detector that comes built in to
Akka. It can be enabled again, if you plan to use this feature (Not recommended). A larger
interval value in seconds reduces network overhead and a smaller value ( ~ 1 s) might be more
informative for Akka's failure detector. Tune this in combination of `spark.akka.heartbeat.pauses`
if you need to. A likely positive use case for using failure detector would be: a sensistive
failure detector can help evict rogue executors quickly. However this is usually not the case
as GC pauses and network lags are expected in a real Spark cluster. Apart from that enabling
this leads to a lot of exchanges of heart beats between nodes leading to flooding the network
This is set to a larger value to disable the transport failure detector that comes built in to
Akka. It can be enabled again, if you plan to use this feature (Not recommended). A larger
interval value in seconds reduces network overhead and a smaller value ( ~ 1 s) might be more
informative for Akka's failure detector. Tune this in combination of `spark.akka.heartbeat.pauses`
if you need to. A likely positive use case for using failure detector would be: a sensistive
failure detector can help evict rogue executors quickly. However this is usually not the case
as GC pauses and network lags are expected in a real Spark cluster. Apart from that enabling
this leads to a lot of exchanges of heart beats between nodes leading to flooding the network
with those.
</td>
</tr>
Expand All @@ -871,7 +871,7 @@ Apart from these, the following properties are also available, and may be useful
<td>6000</td>
<td>
This is set to a larger value to disable the transport failure detector that comes built in to Akka.
It can be enabled again, if you plan to use this feature (Not recommended). Acceptable heart
It can be enabled again, if you plan to use this feature (Not recommended). Acceptable heart
beat pause in seconds for Akka. This can be used to control sensitivity to GC pauses. Tune
this along with `spark.akka.heartbeat.interval` if you need to.
</td>
Expand Down Expand Up @@ -938,9 +938,9 @@ Apart from these, the following properties are also available, and may be useful
</tr>
<tr>
<td><code>spark.network.timeout</code></td>
<td>120</td>
<td>120s</td>
<td>
Default timeout for all network interactions, in seconds. This config will be used in
Default timeout for all network interactions. This config will be used in
place of <code>spark.core.connection.ack.wait.timeout</code>, <code>spark.akka.timeout</code>,
<code>spark.storage.blockManagerSlaveTimeoutMs</code> or
<code>spark.shuffle.io.connectionTimeout</code>, if they are not configured.
Expand Down Expand Up @@ -1215,9 +1215,9 @@ Apart from these, the following properties are also available, and may be useful
</tr>
<tr>
<td><code>spark.core.connection.ack.wait.timeout</code></td>
<td>60</td>
<td>60s</td>
<td>
Number of seconds for the connection to wait for ack to occur before timing
How long for the connection to wait for ack to occur before timing
out and giving up. To avoid unwilling timeout caused by long pause like GC,
you can set larger value.
</td>
Expand Down