Skip to content
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

[BUG] checkValue does not work in RapidsConf #6086

Closed
sinkinben opened this issue Jul 26, 2022 · 0 comments · Fixed by #6116
Closed

[BUG] checkValue does not work in RapidsConf #6086

sinkinben opened this issue Jul 26, 2022 · 0 comments · Fixed by #6116
Labels
bug Something isn't working

Comments

@sinkinben
Copy link
Contributor

sinkinben commented Jul 26, 2022

Describe the bug
In RapidsConf.scala, there are some ConfigBuiler object. For example,

  val MULTITHREAD_READ_NUM_THREADS = conf("spark.rapids.sql.multiThreadedRead.numThreads")
      .doc("The maximum number of threads on each executor to use for reading small " +
        "files in parallel. This can not be changed at runtime after the executor has " +
        ...
        ".")
      .integerConf
      .checkValue(v => v > 0, "The thread count must be greater than zero.")
      .createWithDefault(0)

Here checkValue does not work. createWithDefault(-1) or createWithDefault(0) will not throw any exception.

Even if "spark.rapids.sql.multiThreadedRead.numThreads" = -1 is set explicitly by user, there is no exception, either.


Steps/Code to reproduce bug
Run spark-rapids locally via:

spark-shell --jars ${RAPIDS_JAR} \
  --conf spark.sql.adaptive.enabled=false \
  --conf spark.plugins=com.nvidia.spark.SQLPlugin \
  --conf spark.rapids.sql.multiThreadedRead.numThreads=-1
 
scala> val key = "spark.rapids.sql.multiThreadedRead.numThreads"                                                                    
key: String = spark.rapids.sql.multiThreadedRead.numThreads                                                                         
                                                                                                                                    
scala> spark.conf.get(key)                                                                                                          
res0: String = -1                                                                                                                   
                                                                                                                                    
scala> :quit

Expected behavior
When spark.rapids.sql.multiThreadedRead.numThreads <= 0, we should throw an IllegalArgument exception.

In original spark, if we set an invalid parameter, there is an IllegalArgument.

$ spark-shell --conf spark.sql.shuffle.partitions=-1                                                                                                                              

scala> spark.conf.get("spark.sql.shuffle.partitions")
java.lang.IllegalArgumentException: Error while instantiating 'org.apache.spark.sql.hive.HiveSessionStateBuilder':
  at org.apache.spark.sql.SparkSession$.org$apache$spark$sql$SparkSession$$instantiateSessionState(SparkSession.scala:1178)
  at org.apache.spark.sql.SparkSession.$anonfun$sessionState$2(SparkSession.scala:162)
  at scala.Option.getOrElse(Option.scala:189)
  at org.apache.spark.sql.SparkSession.sessionState$lzycompute(SparkSession.scala:160)
  at org.apache.spark.sql.SparkSession.sessionState(SparkSession.scala:157)
  at org.apache.spark.sql.SparkSession.conf$lzycompute(SparkSession.scala:185)
  at org.apache.spark.sql.SparkSession.conf(SparkSession.scala:185)
  ... 47 elided
Caused by: java.lang.IllegalArgumentException: '-1' in spark.sql.shuffle.partitions is invalid. The value of spark.sql.shuffle.partitions must be positive
...
@sinkinben sinkinben added bug Something isn't working ? - Needs Triage Need team to review and classify labels Jul 26, 2022
@firestarman firestarman changed the title [BUG] checkValue is invalid in RapidsConf [BUG] checkValue does not work in RapidsConf Jul 26, 2022
@sameerz sameerz removed the ? - Needs Triage Need team to review and classify label Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants