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

[GLUTEN-8020][VL] Remove the libhdfs3 installation script required for static linking #8013

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

JkSelf
Copy link
Contributor

@JkSelf JkSelf commented Nov 21, 2024

What changes were proposed in this pull request?

  1. We will only retain the dynamic libraries libhdfs.so or libhdfs3.so at runtime based on this benchmark here. So there is no need to keep the libhdfs3 installation script required for static linking.
  2. Some customers still use libhdfs3. We provide a script to compile libhdfs3.so in the dev folder

How was this patch tested?

Existing compile scripts.

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
Copy link
Contributor

@JkSelf, can we also remove ENABLE_HDFS option from some build scripts?

@zhouyuan
Copy link
Contributor

@JkSelf could you please help to link with the right Issue?

@JkSelf JkSelf changed the title Clean up the libhdfs3 install [GLUTEN-8020] Clean up the libhdfs3 install Nov 22, 2024
@JkSelf JkSelf changed the title [GLUTEN-8020] Clean up the libhdfs3 install [GLUTEN-8020] Remove the libhdfs3 installation script required for static linking Nov 22, 2024
Copy link

#8020

@PHILO-HE
Copy link
Contributor

@JkSelf, can we also remove ENABLE_HDFS option from some build scripts?

Seems we still need it for deciding whether to register hdfs filesystem.

@JkSelf
Copy link
Contributor Author

JkSelf commented Nov 22, 2024

@JkSelf, can we also remove ENABLE_HDFS option from some build scripts?

Seems we still need it for deciding whether to register hdfs filesystem.

Yes. We also need to pass the enable_hdfs to velox compile scripts.

@FelixYBW
Copy link
Contributor

@JkSelf is document updated?

@FelixYBW FelixYBW changed the title [GLUTEN-8020] Remove the libhdfs3 installation script required for static linking [VL][GLUTEN-8020] Remove the libhdfs3 installation script required for static linking Nov 23, 2024
@yabinma
Copy link
Contributor

yabinma commented Nov 26, 2024

The changes is helpful for #7977 , libhdfs3 compile issue found during the test. I will bypass the libhdfs3.

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale stale label Jan 11, 2025
@FelixYBW
Copy link
Contributor

@JkSelf @PHILO-HE will we merge the PR?

we may still need the script to build dynamic libhdfs3.so for perf test. Do we have one?
@zhouyuan

@github-actions github-actions bot removed the stale stale label Jan 14, 2025
@JkSelf JkSelf force-pushed the vcpkg-delete-libhdfs3 branch from 2e3a920 to b1419f2 Compare January 14, 2025 02:59
@JkSelf
Copy link
Contributor Author

JkSelf commented Jan 14, 2025

@JkSelf @PHILO-HE will we merge the PR?

we may still need the script to build dynamic libhdfs3.so for perf test. Do we have one? @zhouyuan

@FelixYBW Yes. We will merge this PR. And we also provide script to build libhdfs3.so in this PR.

@zhouyuan zhouyuan changed the title [VL][GLUTEN-8020] Remove the libhdfs3 installation script required for static linking [GLUTEN-8020][VL] Remove the libhdfs3 installation script required for static linking Jan 14, 2025
@github-actions github-actions bot added the DOCS label Jan 14, 2025
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.

Do we have any tests covering build_libhdfs3.sh?

dev/vcpkg/vcpkg.json Show resolved Hide resolved
@JkSelf
Copy link
Contributor Author

JkSelf commented Jan 14, 2025

Do we have any tests covering build_libhdfs3.sh?

@zhztheplayer Test this script locally.

@zhouyuan zhouyuan merged commit ea58aab into apache:main Jan 14, 2025
46 checks passed
@GlutenPerfBot
Copy link
Contributor

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

query log/native_master_01_14_2025_time.csv log/native_master_01_13_2025_f79f0528bf_time.csv difference percentage
q1 15.96 15.37 -0.583 96.35%
q2 15.23 16.51 1.274 108.36%
q3 4.78 5.62 0.838 117.52%
q4 86.18 85.99 -0.191 99.78%
q5 11.33 14.41 3.081 127.19%
q6 5.09 5.04 -0.049 99.03%
q7 6.44 5.90 -0.545 91.55%
q8 4.99 4.45 -0.542 89.14%
q9 29.20 27.45 -1.754 93.99%
q10 12.26 12.76 0.506 104.13%
q11 41.87 43.71 1.846 104.41%
q12 1.82 2.37 0.555 130.51%
q13 9.43 9.10 -0.331 96.49%
q14a 66.22 60.80 -5.420 91.82%
q14b 57.69 56.62 -1.067 98.15%
q15 3.47 3.41 -0.058 98.34%
q16 28.99 29.42 0.424 101.46%
q17 7.63 7.28 -0.352 95.39%
q18 10.10 10.31 0.218 102.16%
q19 4.12 3.66 -0.457 88.92%
q20 2.31 2.14 -0.167 92.74%
q21 1.86 1.54 -0.314 83.09%
q22 9.60 11.26 1.662 117.31%
q23a 135.90 135.38 -0.514 99.62%
q23b 162.60 163.70 1.099 100.68%
q24a 99.90 99.88 -0.018 99.98%
q24b 99.21 96.72 -2.488 97.49%
q25 7.71 9.99 2.284 129.63%
q26 4.97 5.28 0.311 106.25%
q27 4.91 6.72 1.813 136.92%
q28 37.90 38.75 0.857 102.26%
q29 19.90 19.51 -0.394 98.02%
q30 6.53 8.01 1.487 122.79%
q31 10.13 10.56 0.424 104.19%
q32 2.18 2.30 0.126 105.76%
q33 7.71 8.81 1.103 114.30%
q34 5.79 4.51 -1.271 78.03%
q35 12.06 10.88 -1.178 90.22%
q36 6.22 5.74 -0.476 92.34%
q37 5.20 5.24 0.037 100.72%
q38 25.34 17.48 -7.861 68.98%
q39a 4.43 4.63 0.198 104.47%
q39b 4.95 5.77 0.822 116.63%
q40 6.85 5.35 -1.507 78.01%
q41 0.89 0.87 -0.026 97.13%
q42 2.06 1.19 -0.870 57.69%
q43 4.55 4.73 0.183 104.03%
q44 12.39 11.79 -0.601 95.15%
q45 4.16 4.14 -0.022 99.46%
q46 5.49 5.46 -0.035 99.36%
q47 19.43 21.32 1.882 109.69%
q48 6.56 6.59 0.033 100.51%
q49 10.41 10.04 -0.374 96.41%
q50 41.21 39.74 -1.473 96.43%
q51 14.48 14.40 -0.084 99.42%
q52 1.20 1.21 0.012 101.01%
q53 3.04 3.04 0.002 100.06%
q54 6.79 6.87 0.075 101.10%
q55 2.17 1.29 -0.877 59.54%
q56 7.28 7.09 -0.186 97.44%
q57 12.65 13.54 0.890 107.04%
q58 3.29 3.57 0.280 108.53%
q59 6.34 6.88 0.546 108.62%
q60 7.85 9.93 2.081 126.51%
q61 7.61 9.52 1.910 125.11%
q62 5.51 4.92 -0.593 89.25%
q63 3.28 3.07 -0.209 93.64%
q64 62.43 64.02 1.594 102.55%
q65 30.42 29.16 -1.265 95.84%
q66 4.11 4.69 0.582 114.18%
q67 230.50 226.49 -4.014 98.26%
q68 4.25 4.29 0.041 100.96%
q69 7.06 6.87 -0.193 97.27%
q70 12.54 12.45 -0.086 99.31%
q71 5.40 7.84 2.445 145.29%
q72 39.65 38.78 -0.871 97.80%
q73 3.31 3.98 0.665 120.08%
q74 26.32 28.06 1.745 106.63%
q75 44.39 43.75 -0.642 98.55%
q76 14.63 14.33 -0.301 97.95%
q77 3.91 3.85 -0.065 98.35%
q78 85.13 84.03 -1.100 98.71%
q79 5.13 5.02 -0.107 97.91%
q80 17.33 17.30 -0.026 99.85%
q81 7.98 8.28 0.303 103.79%
q82 10.47 9.87 -0.598 94.29%
q83 3.06 2.53 -0.526 82.79%
q84 3.70 3.75 0.048 101.30%
q85 10.04 9.94 -0.104 98.96%
q86 4.52 4.88 0.363 108.05%
q87 17.97 19.20 1.237 106.89%
q88 24.37 22.97 -1.402 94.25%
q89 4.42 4.31 -0.110 97.52%
q90 3.35 3.27 -0.081 97.60%
q91 6.67 5.21 -1.458 78.15%
q92 2.21 2.57 0.358 116.21%
q93 51.40 54.61 3.204 106.23%
q94 17.54 17.24 -0.305 98.26%
q9 97.12 99.65 2.540 102.62%
q5 3.17 2.97 -0.200 93.69%
q96 28.04 28.18 0.138 100.49%
q97 2.80 3.43 0.631 122.55%
q98 9.81 10.67 0.857 108.74%
q99 9.81 10.67 0.857 108.74%
total 2200.74 2200.01 -0.727 99.97%

@GlutenPerfBot
Copy link
Contributor

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

query log/native_master_01_14_2025_time.csv log/native_master_01_13_2025_f79f0528bf_time.csv difference percentage
q1 44.87 44.74 -0.129 99.71%
q2 44.60 44.44 -0.159 99.64%
q3 91.61 92.10 0.491 100.54%
q4 70.24 70.11 -0.138 99.80%
q5 179.79 178.21 -1.579 99.12%
q6 18.83 18.88 0.053 100.28%
q7 104.68 104.20 -0.481 99.54%
q8 184.88 183.36 -1.520 99.18%
q9 277.97 281.19 3.216 101.16%
q10 102.77 101.09 -1.675 98.37%
q11 34.15 34.24 0.083 100.24%
q12 43.07 42.20 -0.862 98.00%
q13 78.62 75.08 -3.537 95.50%
q14 36.60 35.64 -0.957 97.38%
q15 66.30 65.16 -1.134 98.29%
q16 26.99 27.01 0.017 100.06%
q17 232.07 231.50 -0.571 99.75%
q18 365.90 357.90 -8.000 97.81%
q19 37.35 36.49 -0.861 97.69%
q20 59.66 59.10 -0.563 99.06%
q21 530.94 537.93 6.990 101.32%
q22 24.77 25.01 0.237 100.96%
total 2656.66 2645.58 -11.079 99.58%

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.

7 participants