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.

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

Jump to bottom

[GLUTEN-3559][VL] Fix failing UTs with spark 3.4 #4023

Merged
merged 13 commits into from
Dec 26, 2023
Merged

[GLUTEN-3559][VL] Fix failing UTs with spark 3.4 #4023

merged 13 commits into from
Dec 26, 2023

Conversation

ayushi-agarwal
Copy link
Contributor

What changes were proposed in this pull request?

This PR fixes the following suites:

  1. Gluten Ensure Requirements Suite
  2. GlutenAdaptiveQueryExecSuite
  3. GlutenFallbackSuite
  4. GlutenCTEHintSuite
    Appender tests were failing because in Spark 3.4 log4j-slf4j2-impl is being used instead of log4j-slf4j-impl [SPARK-40511][BUILD][CORE] Upgrade slf4j to 2.0.3 spark#37844

(Fixes: #3559)

How was this patch tested?

Ran these UTs to confirm the fix

@github-actions GitHub Actions
Copy link

#3559

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@ayushi-agarwal
Copy link
Contributor Author

CC: @JkSelf

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@JkSelf
Copy link
Contributor

JkSelf commented Dec 13, 2023

@ayushi-agarwal Thanks for your working. It appears that some test suites are still failing. Could you please assist in investigating?

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@ayushi-agarwal
Copy link
Contributor Author

@JkSelf I have rebased and fixed the error.

JkSelf
JkSelf reviewed Dec 14, 2023
View reviewed changes
Copy link
Contributor

@JkSelf JkSelf left a comment

Choose a reason for hiding this comment

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

@ayushi-agarwal LGTM. Just two small questions. Thanks.

gluten-ut/spark34/src/test/scala/org/apache/spark/sql/gluten/GlutenFallbackSuite.scala Show resolved Hide resolved
JkSelf
JkSelf reviewed Dec 14, 2023
View reviewed changes
...4/src/test/scala/org/apache/spark/sql/execution/exchange/GlutenEnsureRequirementsSuite.scala Outdated
@@ -42,4 +48,57 @@ class GlutenEnsureRequirementsSuite extends EnsureRequirementsSuite with GlutenS
assert(res.queryExecution.executedPlan.collect { case s: ShuffleExchangeLike => s }.size == 2)
}
}

test(GLUTEN_TEST + "SPARK-41986: Introduce shuffle on SinglePartition") {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ayushi-agarwal What is the reason behind rewriting this suite? Is it solely due to the addition of the configuration SQLConf.SHUFFLE_PARTITIONS.key -> "5"?

Copy link
Contributor Author

@ayushi-agarwal ayushi-agarwal Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the test above this is also rewritten because of it. I think now both test cases can be removed from this file as the config has been added in line 34 now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JkSelf I have removed it.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

JkSelf
JkSelf reviewed Dec 19, 2023
View reviewed changes
Copy link
Contributor

@JkSelf JkSelf left a comment

Choose a reason for hiding this comment

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

LGTM. And waiting CI pass.

@JkSelf
Copy link
Contributor

JkSelf commented Dec 19, 2023

@ayushi-agarwal There exists compile issues. Can you help to rebase? Thanks.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@ayushi-agarwal
Copy link
Contributor Author

@ayushi-agarwal There exists compile issues. Can you help to rebase? Thanks.

@JkSelf I have rebased it.

@ayushi-agarwal
Copy link
Contributor Author

cc @zhouyuan @JkSelf The build is green now.

@JkSelf
Copy link
Contributor

JkSelf commented Dec 25, 2023

cc @zhouyuan @JkSelf The build is green now.

Yes. You may need to rebase again. Thanks.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@ayushi-agarwal
Copy link
Contributor Author

@JkSelf I have rebased it again. Could you please approve it so that it gets merged once the build is green?

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@ayushi-agarwal
Copy link
Contributor Author

@JkSelf All 18 checks have passed now, could you please approve it?

JkSelf
JkSelf approved these changes Dec 26, 2023
View reviewed changes
Copy link
Contributor

@JkSelf JkSelf left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@JkSelf JkSelf merged commit 168a6d9 into apache:main Dec 26, 2023
18 checks passed
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_4023_time.csv log/native_master_12_26_2023_3406f79b0_time.csv difference percentage
q1 32.76 31.66 -1.096 96.66%
q2 25.91 25.98 0.062 100.24%
q3 38.02 37.79 -0.231 99.39%
q4 39.62 39.69 0.075 100.19%
q5 71.77 71.91 0.136 100.19%
q6 7.08 6.93 -0.146 97.94%
q7 87.39 86.42 -0.970 98.89%
q8 85.34 86.45 1.110 101.30%
q9 126.21 122.77 -3.440 97.27%
q10 46.13 43.40 -2.728 94.08%
q11 19.92 20.24 0.322 101.62%
q12 26.51 26.94 0.426 101.61%
q13 45.90 46.42 0.519 101.13%
q14 17.83 19.81 1.979 111.10%
q15 27.25 28.19 0.938 103.44%
q16 15.48 15.18 -0.298 98.08%
q17 104.53 102.38 -2.152 97.94%
q18 152.18 148.55 -3.625 97.62%
q19 13.18 13.55 0.367 102.78%
q20 26.89 26.89 0.006 100.02%
q21 228.19 227.87 -0.323 99.86%
q22 14.04 14.03 -0.010 99.93%
total 1252.12 1243.04 -9.078 99.28%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JkSelf JkSelf JkSelf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[VL] Track all the failed unit test in Spark 3.4.
3 participants
@ayushi-agarwal @JkSelf @GlutenPerfBot