Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion core/src/main/scala/org/apache/spark/deploy/Client.scala
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ object Client {
val conf = new SparkConf()
val driverArgs = new ClientArguments(args)

conf.set("spark.rpc.askTimeout", "10")
if (!conf.contains("spark.rpc.askTimeout")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @zsxwing was there a reason this timeout is hard coded here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess part of the reason is that master/client have low memory usage and as a result unlikely to have long timeouts due to gc.

@srowen I'm not sure if this is a good change.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know. @pwendell added it in 3d939e5 and changed the value to 10 in another commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, does that mean that the default should be 10s? the real issue is just that the default ends up being "10" instead of advertised 120s.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't actually impact the default for Spark apps, but only the standalone cluster doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's right. I suspect that either it's OK to let them use the default timeout of 120s in the end, or, probably bears noting the different practical default of 10s in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea maybe we should just note it with a comment explaining why it is 10s.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, see my current version, that at least lets spark.rpc.askTimeout be set to something besides 10s in standalone. See https://issues.apache.org/jira/browse/SPARK-18353 discussion too.

Copy link
Contributor

Choose a reason for hiding this comment

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

can this ever be set? I don't remember whether the standalone master/worker reads config files or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

also we should probably add this to config/package.scala so we don't duplicate it

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, to move forward on this: @rxin I think this ends up affecting application launch because this is the bit that runs applications rather than starts workers, if that's what you mean. It will process --conf from the user command line but then override it here. The report in the JIRA seems to confirm that. At the least, this change would not affect the current behavior.

@andrewor14 that makes sense, though I note that the other spark.rpc configs are not in config/package.scala. The ConfigBuilder doesn't quite seem able to express a property whose default is another property either. Should I still try to refactor that?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's a separate issue so we can fix it later, but we should fix it since we're just duplicating strings

conf.set("spark.rpc.askTimeout", "10s")
}
Logger.getRootLogger.setLevel(driverArgs.logLevel)

val rpcEnv =
Expand Down
4 changes: 2 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ Apart from these, the following properties are also available, and may be useful
</tr>
<tr>
<td><code>spark.rpc.askTimeout</code></td>
<td>120s</td>
<td><code>spark.network.timeout</code></td>
<td>
Duration for an RPC ask operation to wait before timing out.
</td>
Expand Down Expand Up @@ -1531,7 +1531,7 @@ 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>60s</td>
<td><code>spark.network.timeout</code></td>
<td>
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,
Expand Down