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-4917][VL] GHA with pre-built docker image and github runner #4936

Merged
merged 44 commits into from
Mar 25, 2024
Merged

[GLUTEN-4917][VL] GHA with pre-built docker image and github runner #4936

merged 44 commits into from
Mar 25, 2024

Conversation

zhouyuan
Copy link
Contributor

@zhouyuan zhouyuan commented Mar 12, 2024 •
edited

What changes were proposed in this pull request?

allow to use pre-built docker image in GHA
running in github runner

gap:

  • cpp benchmark, HBM tests
  • celeborn tests
  • oom/randome kill tests
  • iceberg/delta lake tests

How was this patch tested?

pass GHA

@github-actions GitHub Actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

2 similar comments
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan zhouyuan force-pushed the wip_ci_apache_jenkins branch 2 times, most recently from 9d0717e to 3d1119d Compare March 13, 2024 09:41
@apache apache deleted a comment from github-actions bot Mar 13, 2024
@apache apache deleted a comment from github-actions bot Mar 13, 2024
@apache apache deleted a comment from github-actions bot Mar 13, 2024
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan zhouyuan force-pushed the wip_ci_apache_jenkins branch 2 times, most recently from f9cb288 to 46f58e5 Compare March 13, 2024 23:30
@apache apache deleted a comment from github-actions bot Mar 13, 2024
@apache apache deleted a comment from github-actions bot Mar 13, 2024
@apache apache deleted a comment from github-actions bot Mar 14, 2024
@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

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@apache apache deleted a comment from github-actions bot Mar 24, 2024
@apache apache deleted a comment from github-actions bot Mar 24, 2024
@apache apache deleted a comment from github-actions bot Mar 24, 2024
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan
fix maven path
2b87a8e 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan
ignore iceberge/delta lake tests in spark 33/34
dd8716b 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan
enable udf tests
15c07d7 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan
enable more tests
5609145 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@apache apache deleted a comment from github-actions bot Mar 24, 2024
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan
adding celeborn test
a78551c 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan
ban flaky ut
48eeb2c 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan
disable celeborn; disable random kill test
941655e 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@zhouyuan
adding back old tests; deprecate old velox tests
3a20136 
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan zhouyuan changed the title [DNM][VL] GHA with pre-built docker image and github runner [GLUTEN-4917][VL] GHA with pre-built docker image and github runner Mar 24, 2024
@zhouyuan zhouyuan marked this pull request as ready for review March 24, 2024 23:08
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI

@github-actions GitHub Actions
Copy link

#4917

@zhouyuan
Copy link
Contributor Author

@PHILO-HE

zhouyuan
zhouyuan commented Mar 25, 2024
View reviewed changes
gluten-ut/spark33/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala
@@ -71,7 +71,7 @@ class VeloxTestSettings extends BackendTestSettings {
enableSuite[GlutenWriterColumnarRulesSuite]

enableSuite[GlutenQueryCompilationErrorsDSv2Suite]
enableSuite[GlutenQueryCompilationErrorsSuite]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The suite seems buggy on new runner, but only for Spark33. Looks odd, so I disabled it first

2024-03-20T05:47:06.2483725Z �[31m- UNSUPPORTED_FEATURE: Using Python UDF with unsupported join condition *** FAILED ***�[0m
2024-03-20T05:47:06.2486680Z �[31m  Expected exception org.apache.spark.sql.AnalysisException to be thrown, but java.lang.RuntimeException was thrown (QueryCompilationErrorsSuite.scala:144)�[0m

.github/workflows/velox_docker.yml
&& GLUTEN_IT_JVM_ARGS=-Xmx5G sbin/gluten-it.sh queries-compare \
--local --preset=velox --benchmark-type=ds --error-on-memleak --off-heap-size=10g -s=1.0 --threads=16 --iterations=1

# run-tpc-test-centos8-oom-randomkill:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reports OOM on current GH runner, need to run with proper config

.github/workflows/velox_docker.yml
export SPARK_SCALA_VERSION=2.12 && \
export MAVEN_HOME=/usr/lib/maven
export PATH=${PATH}:${MAVEN_HOME}/bin
mvn clean install -Pspark-3.3 -Pbackends-velox -Prss -Pspark-ut -DargLine="-Dspark.test.home=$GITHUB_WORKSPACE//shims/spark33/spark_home/" -DtagsToExclude=org.apache.spark.tags.ExtendedSQLTest,io.glutenproject.tags.UDFTest,io.glutenproject.tags.SkipTestTags && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

disabled iceberg test due to some version-hint.txt missing
disabled delta tests due to heap OOM

zhztheplayer
zhztheplayer approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

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

Thanks!

@zhouyuan zhouyuan merged commit 25cd1f5 into apache:main Mar 25, 2024
32 of 33 checks passed
@zhouyuan
Copy link
Contributor Author

Hi @ulysses-you @zwangsheng
we just find the the golden file tests are not covered, could you please help to re-enable this, if this is still required?

thanks,
-yuan

@ulysses-you
Copy link
Contributor

Is there any block on enabling golden files test ? If not, I can help to recover it.

@zhouyuan
Copy link
Contributor Author

Is there any block on enabling golden files test ? If not, I can help to recover it.

https://github.com/apache/incubator-gluten/blob/main/.github/workflows/velox_be.yml.deprecated#L104

If I understand correctly, we may just need to upload the artifact now, the "docker cp" is not required

thanks,
-yuan

@PHILO-HE PHILO-HE mentioned this pull request Mar 26, 2024
Merged
@zhztheplayer zhztheplayer mentioned this pull request Apr 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhztheplayer zhztheplayer zhztheplayer 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.

None yet
3 participants
@zhouyuan @ulysses-you @zhztheplayer