Skip to content

[SPARK-26564] Fix wrong assertions and error messages for parameter checking #23488

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.

Sign up for GitHub

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

Closed
sekikn wants to merge 5 commits into from
Closed

[SPARK-26564] Fix wrong assertions and error messages for parameter checking #23488

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab9ec65
[SPARK-26564] Fix misleading error message about spark.network.timeou…
sekikn Jan 7, 2019
73fd0a5
Merge remote-tracking branch 'upstream/master' into SPARK-26564
sekikn Jan 10, 2019
b184363
[SPARK-26564] Fix wrong assertions and error messages for parameter c…
sekikn Jan 10, 2019
cc56479
Merge remote-tracking branch 'upstream/master' into SPARK-26564
sekikn Jan 12, 2019
22a69d1
[SPARK-26564] Fix wrong assertions and error messages for parameter c…
sekikn Jan 12, 2019
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
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/SparkConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Seria
// If spark.executor.heartbeatInterval bigger than spark.network.timeout,
// it will almost always cause ExecutorLostFailure. See SPARK-22754.
require(executorTimeoutThresholdMs > executorHeartbeatIntervalMs, "The value of " +
s"spark.network.timeout=${executorTimeoutThresholdMs}ms must be no less than the value of " +
s"spark.network.timeout=${executorTimeoutThresholdMs}ms must be greater than the value of " +
s"spark.executor.heartbeatInterval=${executorHeartbeatIntervalMs}ms.")
}

Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/optim/WeightedLeastSquares.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private[ml] class WeightedLeastSquares(
require(regParam >= 0.0, s"regParam cannot be negative: $regParam")
require(elasticNetParam >= 0.0 && elasticNetParam <= 1.0,
s"elasticNetParam must be in [0, 1]: $elasticNetParam")
require(maxIter >= 0, s"maxIter must be a positive integer: $maxIter")
require(maxIter > 0, s"maxIter must be a positive integer: $maxIter")
require(tol >= 0.0, s"tol must be >= 0, but was set to $tol")

/**
Expand Down
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private[execution] final class LongToUnsafeRowMap(val mm: TaskMemoryManager, cap

private def init(): Unit = {
if (mm != null) {
require(capacity < 512000000, "Cannot broadcast more than 512 millions rows")
require(capacity <= 512000000, "Cannot broadcast more than 512 millions rows")

@HyukjinKwon HyukjinKwon Jan 11, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the previous condition was true.

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/BroadcastExchangeExec.scala

Lines 80 to 83 in 79c6689

if (numRows >= 512000000) {
throw new SparkException(
s"Cannot broadcast the table with more than 512 millions rows: $numRows rows")
}

negated numRows >= 512000000 should be capacity < 512000000

@srowen srowen Jan 11, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, good catch. Sorry @sekikn could we change this one more time, to use < and then fix the error message instead?

@sekikn sekikn Jan 12, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'm going to update the PR.

var n = 1
while (n < capacity) n *= 2
ensureAcquireMemory(n * 2L * 8 + (1 << 20))
Expand Down