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

[VL] Add a docker build job and reuse pre-built arrow libs #6826

Merged
merged 13 commits into from
Aug 15, 2024

Conversation

PHILO-HE
Copy link
Contributor

@PHILO-HE PHILO-HE commented Aug 14, 2024

  1. Add a docker build job with the help of GHA. The DOCKERHUB_USER & DOCKERHUB_TOKEN secrets already set are used to login docker hub and push docker image. But these secrets are not available in pr from forked repo, which makes this pr's docker build job red.
  2. Use the new docker in CI checks. As arrow libs (static lib, jar) are installed in docker image, build_arrow option can be turned off.

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:

@PHILO-HE PHILO-HE force-pushed the build-docker-upstream branch from 7e523b0 to 8d0a771 Compare August 14, 2024 03:00
@PHILO-HE PHILO-HE force-pushed the build-docker-upstream branch from 8d0a771 to 9de4b10 Compare August 14, 2024 07:01
@@ -51,7 +51,7 @@ concurrency:
jobs:
build-native-lib-centos-7:
runs-on: ubuntu-20.04
container: apache/gluten:gluten-vcpkg-builder_2024_08_05 # centos7 with dependencies installed
container: apache/gluten:vcpkg-centos-7
Copy link
Member

Choose a reason for hiding this comment

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

Are we planning to do the same on centos 8 build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhztheplayer, yes, it would be better to also do this for centos-8 build. At least, those libs generated by arrow build (cpp/java) can be cached in docker to accelerate CI job when the current folder based cache miss happens.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we reuse the binary on centos 7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhouyuan, I will try this.

rui-mo
rui-mo previously approved these changes Aug 15, 2024
This reverts commit 3ba2feb.
@PHILO-HE PHILO-HE merged commit 1845f30 into apache:main Aug 15, 2024
6 of 7 checks passed
@GlutenPerfBot
Copy link
Contributor

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

query log/native_master_08_15_2024_time.csv log/native_master_08_14_2024_1b619a0388_time.csv difference percentage
q1 40.51 39.02 -1.496 96.31%
q2 30.39 29.62 -0.770 97.46%
q3 53.23 51.65 -1.572 97.05%
q4 44.25 42.92 -1.331 96.99%
q5 102.93 103.89 0.968 100.94%
q6 12.09 11.18 -0.906 92.51%
q7 116.78 115.76 -1.020 99.13%
q8 117.10 115.92 -1.181 98.99%
q9 169.16 168.25 -0.910 99.46%
q10 66.73 65.66 -1.075 98.39%
q11 26.55 25.87 -0.683 97.43%
q12 30.12 30.71 0.591 101.96%
q13 51.22 51.84 0.623 101.22%
q14 24.79 26.31 1.518 106.12%
q15 54.28 53.60 -0.675 98.76%
q16 18.18 18.19 0.007 100.04%
q17 130.17 130.87 0.694 100.53%
q18 196.71 196.20 -0.510 99.74%
q19 27.00 28.20 1.195 104.43%
q20 47.52 43.48 -4.041 91.50%
q21 376.64 384.72 8.076 102.14%
q22 15.77 15.41 -0.367 97.67%
total 1752.12 1749.25 -2.867 99.84%

# See the License for the specific language governing permissions and
# limitations under the License.

name: Build and Push Docker Image
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please help to add one doc on how to use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhouyuan, I will do that. Thanks!

@@ -2,12 +2,9 @@

set -e

yum install sudo patch java-1.8.0-openjdk-devel -y
Copy link
Contributor

Choose a reason for hiding this comment

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

Initially this is kept in case someone is using this script from a clean centos 7 image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhouyuan, maybe we can just tell user to refer weekly build job for building from clean image. centos-7/8/9, ubuntu-20.04/22.04 are covered in that job. I will document this guide. Thanks!

sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
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.

5 participants