[SPARK-52597][SS][TESTS] Fix the execution failure of StateStoreBasicOperationsBenchmark
#51304
Conversation
| val storeConf = new StateStoreConf(sqlConf) | ||
|
|
||
| val configuration = new Configuration | ||
| configuration.set(StreamExecution.RUN_ID_KEY, UUID.randomUUID().toString) |
There was a problem hiding this comment.
This modification is also needed; otherwise, the execution will still encounter errors:
build/sbt "sql/Test/runMain org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark"
[error] Exception in thread "main" java.lang.AssertionError: assertion failed
[error] at scala.Predef$.assert(Predef.scala:264)
[error] at org.apache.spark.sql.execution.streaming.state.StateStoreProvider$.getRunId(StateStore.scala:673)
[error] at org.apache.spark.sql.execution.streaming.state.RocksDBStateStoreProvider.init(RocksDBStateStoreProvider.scala:394)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark$.newRocksDBStateProvider(StateStoreBasicOperationsBenchmark.scala:484)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark$.$anonfun$runPutBenchmark$3(StateStoreBasicOperationsBenchmark.scala:92)
[error] at scala.runtime.java8.JFunction1$mcVI$sp.apply(JFunction1$mcVI$sp.scala:18)
[error] at scala.collection.immutable.List.foreach(List.scala:334)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark$.$anonfun$runPutBenchmark$2(StateStoreBasicOperationsBenchmark.scala:87)
[error] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[error] at org.apache.spark.benchmark.BenchmarkBase.runBenchmark(BenchmarkBase.scala:42)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark$.runPutBenchmark(StateStoreBasicOperationsBenchmark.scala:83)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark$.runBenchmarkSuite(StateStoreBasicOperationsBenchmark.scala:55)
[error] at org.apache.spark.benchmark.BenchmarkBase.main(BenchmarkBase.scala:72)
[error] at org.apache.spark.sql.execution.benchmark.StateStoreBasicOperationsBenchmark.main(StateStoreBasicOperationsBenchmark.scala)
There was a problem hiding this comment.
Actually according to the original comment, there should be a random runid set when it's not found:
There was a problem hiding this comment.
If according to the comment, maybe we should generate a random one there and do a logWarning, so we could remove this random generation at test time?
There was a problem hiding this comment.
Based on the description, it seems so, but the code implementation doesn't appear to match. It looks like when @ericm-db added the assertions, the comments weren't modified accordingly.
@ericm-db Maybe should submit a follow-up for SPARK-52188 to fix this comment?
There was a problem hiding this comment.
If according to the comment, maybe we should generate a random one there and do a logWarning, so we could remove this random generation at test time?
Based on the submission timestamps, it appears that the assertions were added after the comments. Since I'm not very familiar with this part of the code, I hope the original author, @ericm-db, can come up with a solution.
There was a problem hiding this comment.
It seems that in the original logic, only test scenarios would automatically generate a UUID. So, personally, I think we should set it manually here, and the comment is already outdated. WDYT? @WweiL
There was a problem hiding this comment.
I see, thank you for the explanation! Yea I agree with you, let me just do a follow up on the comment there
There was a problem hiding this comment.
Thank you @WweiL ~
There was a problem hiding this comment.
Can you merge this, thank you!
There was a problem hiding this comment.
Can you merge this, thank you!
done
|
I've submitted a job that executes via GitHub Action. Let's verify the effect of the modifications:
|
### What changes were proposed in this pull request? #50924 removed the logic of generating a random id in `StateStoreProvider.getRunId` didn't update the comment. Following discussion here, update the comment #51304 (comment) ### Why are the changes needed? Code readability ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No need ### Was this patch authored or co-authored using generative AI tooling? No Closes #51307 from WweiL/SPARK-52188-followup. Authored-by: Wei Liu <[email protected]> Signed-off-by: yangjie01 <[email protected]>
|
Merged into master. Thanks @HeartSaVioR @zecookiez and @WweiL |
What changes were proposed in this pull request?
This pr has made the following changes to fix the
StateStoreBasicOperationsBenchmark:StateStoreConf.RocksDBStateStoreProvider, populate theStreamExecution.RUN_ID_KEYfor the incoming HadoopConfiguration.Why are the changes needed?
Fix the execution failure of
StateStoreBasicOperationsBenchmark:Does this PR introduce any user-facing change?
No
How was this patch tested?
StateStoreBasicOperationsBenchmarkcan be executed successfully.Was this patch authored or co-authored using generative AI tooling?
No