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

[Improvement] Rename Kyuubi's StageInfo to SparkStageInfo to fix class mismatch #5184

Closed
wants to merge 2 commits into from

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Aug 21, 2023

Why are the changes needed?

  • Fix class mismatch when trying to compilation on Scala 2.13, due to implicit class reference to StageInfo. The compilation fails by type mismatching, if the compiler classloader loads Spark's org.apache.spark.schedulerStageInfo ahead of Kyuubi's org.apache.spark.kyuubi.StageInfo.
  • Change var integer to AtomicInteger for numActiveTasks and numCompleteTasks, preventing possible concurrent inconsistency
[ERROR] [Error] /Users/bw/dev/kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala:56: type mismatch;
 found   : java.util.concurrent.ConcurrentHashMap[org.apache.spark.kyuubi.StageAttempt,org.apache.spark.scheduler.StageInfo]
 required: java.util.concurrent.ConcurrentHashMap[org.apache.spark.kyuubi.StageAttempt,org.apache.spark.kyuubi.StageInfo]
[INFO] [Info] : java.util.concurrent.ConcurrentHashMap[org.apache.spark.kyuubi.StageAttempt,org.apache.spark.scheduler.StageInfo] <: java.util.concurrent.ConcurrentHashMap[org.apache.spark.kyuubi.StageAttempt,org.apache.spark.kyuubi.StageInfo]?
[INFO] [Info] : false
[ERROR] [Error] /Users/bw/dev/kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala:126: not enough arguments for constructor StageInfo: (stageId: Int, attemptId: Int, name: String, numTasks: Int, rddInfos: Seq[org.apache.spark.storage.RDDInfo], parentIds: Seq[Int], details: String, taskMetrics: org.apache.spark.executor.TaskMetrics, taskLocalityPreferences: Seq[Seq[org.apache.spark.scheduler.TaskLocation]], shuffleDepId: Option[Int], resourceProfileId: Int, isPushBasedShuffleEnabled: Boolean, shuffleMergerCount: Int): org.apache.spark.scheduler.StageInfo.
Unspecified value parameters name, numTasks, rddInfos...
[ERROR] [Error] /Users/bw/dev/kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala:148: value numActiveTasks is not a member of org.apache.spark.scheduler.StageInfo
[ERROR] [Error] /Users/bw/dev/kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala:156: value numActiveTasks is not a member of org.apache.spark.scheduler.StageInfo
[ERROR] [Error] /Users/bw/dev/kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SQLOperationListener.scala:158: value numCompleteTasks is not a member of org.apache.spark.scheduler.StageInfo

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No.

@bowenliang123
Copy link
Contributor Author

Related to a feature of the progress bar output introduced in #2162. cc @cxzl25

@bowenliang123 bowenliang123 requested review from cxzl25 and removed request for cxzl25 August 21, 2023 09:09
@codecov-commenter
Copy link

Codecov Report

Merging #5184 (d410491) into master (7979aaf) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master   #5184   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         566     566           
  Lines       31703   31703           
  Branches     4134    4134           
======================================
  Misses      31703   31703           
Files Changed Coverage Δ
...org/apache/spark/kyuubi/SQLOperationListener.scala 0.00% <0.00%> (ø)
.../apache/spark/kyuubi/SparkConsoleProgressBar.scala 0.00% <0.00%> (ø)
...in/scala/org/apache/spark/kyuubi/StageStatus.scala 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bowenliang123 bowenliang123 self-assigned this Aug 21, 2023
@bowenliang123 bowenliang123 added this to the v1.8.0 milestone Aug 21, 2023
@bowenliang123
Copy link
Contributor Author

Thanks, merged to master.

@bowenliang123 bowenliang123 deleted the spark-stage-info branch August 21, 2023 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants