Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
morningman merged PR #60519: URL: https://github.com/apache/doris/pull/60519 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
hello-stephen commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3853622755 # BE Regression && UT Coverage Report Increment line coverage `100% (0/0)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/60519_3624c195b20781d0dfa21019cb0ad8e2f33d99a1_merge/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/60519_3624c195b20781d0dfa21019cb0ad8e2f33d99a1_merge/report/index.html) | Category | Coverage | |---|| | Function Coverage | 71.58% (25858/36124) | | Line Coverage | 54.22% (270062/498090) | | Region Coverage | 51.75% (225064/434878) | | Branch Coverage | 53.16% (96525/181570) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
hello-stephen commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3853285047 # BE UT Coverage Report Increment line coverage ` ` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/3624c195b20781d0dfa21019cb0ad8e2f33d99a1_3624c195b20781d0dfa21019cb0ad8e2f33d99a1/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/3624c195b20781d0dfa21019cb0ad8e2f33d99a1_3624c195b20781d0dfa21019cb0ad8e2f33d99a1/report/index.html) | Category | Coverage | |---|| | Function Coverage | 52.55% (19372/36861) | | Line Coverage | 36.04% (179925/499278) | | Region Coverage | 32.39% (139431/430460) | | Branch Coverage | 33.40% (60407/180838) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
doris-robot commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3852983068 ClickBench: Total hot run time: 28.78 s ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools ClickBench test result on commit 3624c195b20781d0dfa21019cb0ad8e2f33d99a1, data reload: false query1 0.050.050.06 query2 0.100.040.05 query3 0.260.090.08 query4 1.610.120.11 query5 0.260.250.24 query6 1.170.680.66 query7 0.030.030.03 query8 0.050.040.04 query9 0.570.510.48 query10 0.560.560.54 query11 0.150.100.10 query12 0.150.100.10 query13 0.640.600.62 query14 1.071.051.05 query15 0.870.870.86 query16 0.390.390.40 query17 1.141.141.13 query18 0.230.210.21 query19 2.092.012.03 query20 0.020.020.01 query21 15.43 0.290.16 query22 5.150.060.06 query23 16.18 0.290.11 query24 1.490.760.63 query25 0.120.080.11 query26 0.150.140.13 query27 0.100.050.08 query28 5.331.150.97 query29 12.54 3.973.19 query30 0.280.130.11 query31 2.820.650.40 query32 3.230.580.49 query33 3.223.253.22 query34 15.98 5.384.78 query35 4.814.784.80 query36 0.660.510.49 query37 0.100.070.06 query38 0.070.050.05 query39 0.050.040.04 query40 0.190.160.16 query41 0.090.040.03 query42 0.040.030.03 query43 0.050.040.03 Total cold run time: 99.49 s Total hot run time: 28.78 s ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
doris-robot commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3852874961 TPC-H: Total hot run time: 31286 ms ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools Tpch sf100 test result on commit 3624c195b20781d0dfa21019cb0ad8e2f33d99a1, data reload: false -- Round 1 -- q1 17649 445442894289 q2 2067361 232 232 q3 10100 1266707 707 q4 10195 830 314 314 q5 7522223318641864 q6 199 178 144 144 q7 879 726 589 589 q8 9277143410891089 q9 5198481747704770 q10 6766195415791579 q11 529 285 285 285 q12 353 370 221 221 q13 17788 408032353235 q14 239 239 220 220 q15 882 819 790 790 q16 657 674 616 616 q17 637 777 510 510 q18 6733653172236531 q19 12511074651 651 q20 400 375 249 249 q21 2968239821082108 q22 385 319 293 293 Total cold run time: 102674 ms Total hot run time: 31286 ms - Round 2, with runtime_filter_mode=off - q1 4511450647404506 q2 258 363 279 279 q3 2294296524012401 q4 1474185913861386 q5 4594445945584459 q6 238 185 139 139 q7 2046188318131813 q8 2593247624142414 q9 7724730375147303 q10 2967305225762576 q11 579 481 474 474 q12 681 829 669 669 q13 3896433635683568 q14 316 298 270 270 q15 828 805 782 782 q16 652 690 642 642 q17 1080131012481248 q18 7705746073507350 q19 843 782 799 782 q20 1965200918671867 q21 4616425240684068 q22 557 550 493 493 Total cold run time: 52417 ms Total hot run time: 49489 ms ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
github-actions[bot] commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3852711987 PR approved by at least one committer and no changes requested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
github-actions[bot] commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3852712218 PR approved by anyone and no changes requested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
xyla commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3852682730 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
hello-stephen commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3851272245 # BE Regression && UT Coverage Report Increment line coverage `100% (0/0)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/60519_9fb89381e030744c65fafc1d747a6ec831ea8fe2_merge/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/60519_9fb89381e030744c65fafc1d747a6ec831ea8fe2_merge/report/index.html) | Category | Coverage | |---|| | Function Coverage | 71.60% (25865/36122) | | Line Coverage | 54.22% (270022/498025) | | Region Coverage | 51.78% (225150/434841) | | Branch Coverage | 53.15% (96482/181542) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
hello-stephen commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3851072743 # BE UT Coverage Report Increment line coverage ` ` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/9fb89381e030744c65fafc1d747a6ec831ea8fe2_9fb89381e030744c65fafc1d747a6ec831ea8fe2/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/9fb89381e030744c65fafc1d747a6ec831ea8fe2_9fb89381e030744c65fafc1d747a6ec831ea8fe2/report/index.html) | Category | Coverage | |---|| | Function Coverage | 52.55% (19369/36858) | | Line Coverage | 36.03% (179873/499220) | | Region Coverage | 32.40% (139462/430425) | | Branch Coverage | 33.40% (60389/180812) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
doris-robot commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3851022053 ClickBench: Total hot run time: 28.72 s ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools ClickBench test result on commit 9fb89381e030744c65fafc1d747a6ec831ea8fe2, data reload: false query1 0.060.060.06 query2 0.090.050.06 query3 0.260.090.09 query4 1.610.110.12 query5 0.260.260.25 query6 1.170.670.68 query7 0.040.040.03 query8 0.060.040.04 query9 0.580.510.49 query10 0.540.540.56 query11 0.150.100.10 query12 0.150.110.11 query13 0.630.620.63 query14 1.071.051.06 query15 0.870.860.87 query16 0.410.400.40 query17 1.111.121.10 query18 0.230.210.21 query19 2.071.972.06 query20 0.030.020.01 query21 15.41 0.240.15 query22 5.260.060.05 query23 15.99 0.300.11 query24 1.440.690.59 query25 0.100.090.05 query26 0.140.130.15 query27 0.060.080.05 query28 4.081.160.96 query29 12.57 3.893.15 query30 0.310.160.15 query31 2.820.670.40 query32 3.240.590.50 query33 3.213.233.30 query34 16.04 5.364.74 query35 4.824.804.89 query36 0.640.500.49 query37 0.110.080.08 query38 0.080.040.04 query39 0.050.030.03 query40 0.200.170.16 query41 0.090.040.03 query42 0.040.030.03 query43 0.050.040.04 Total cold run time: 98.14 s Total hot run time: 28.72 s ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
doris-robot commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3850978086 TPC-H: Total hot run time: 31212 ms ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools Tpch sf100 test result on commit 9fb89381e030744c65fafc1d747a6ec831ea8fe2, data reload: false -- Round 1 -- q1 17596 451243324332 q2 2007377 230 230 q3 10138 1269746 746 q4 10224 886 313 313 q5 7551217418641864 q6 192 180 151 151 q7 860 731 612 612 q8 9265139110921092 q9 5197483848534838 q10 6853195715441544 q11 492 288 301 288 q12 332 373 225 225 q13 17792 402832163216 q14 230 235 223 223 q15 891 835 808 808 q16 686 683 609 609 q17 640 849 434 434 q18 6619658266386582 q19 12421011653 653 q20 404 353 259 259 q21 2702212219101910 q22 353 322 283 283 Total cold run time: 102266 ms Total hot run time: 31212 ms - Round 2, with runtime_filter_mode=off - q1 4359433743684337 q2 259 344 255 255 q3 2110257222732273 q4 1398174413581358 q5 4462442245574422 q6 203 181 139 139 q7 1873184820191848 q8 2621250525812505 q9 7624784376137613 q10 2870302226132613 q11 558 474 452 452 q12 651 754 605 605 q13 3957434035873587 q14 302 303 298 298 q15 895 813 797 797 q16 680 738 690 690 q17 1212142513971397 q18 8308810277807780 q19 908 866 816 816 q20 2148222020402040 q21 4525423841344134 q22 614 553 501 501 Total cold run time: 52537 ms Total hot run time: 50460 ms ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
Copilot commented on code in PR #60519:
URL: https://github.com/apache/doris/pull/60519#discussion_r2766904264
##
thirdparty/build-thirdparty.sh:
##
@@ -551,6 +551,9 @@ build_snappy() {
check_if_source_exist "${SNAPPY_SOURCE}"
cd "${TP_SOURCE_DIR}/${SNAPPY_SOURCE}"
+# Enable RTTI for snappy (required by Doris BE for SnappySlicesSource
inheritance)
+sed -i 's/-fno-rtti/-frtti/g' CMakeLists.txt
Review Comment:
The sed command to enable RTTI is executed every time the build function
runs. While this works because the source is only unpacked once, there's a
potential issue: if someone manually modifies CMakeLists.txt and then reruns
the build, the sed command could fail or have unexpected behavior if the
pattern no longer matches.
Consider making this more robust by either:
1. Checking if the modification has already been applied before running sed
2. Applying this change as a proper patch in download-thirdparty.sh (similar
to how other libraries are patched)
Example for option 1:
```bash
if grep -q '\-fno-rtti' CMakeLists.txt; then
sed -i 's/-fno-rtti/-frtti/g' CMakeLists.txt
fi
```
Option 2 would be more consistent with how other libraries (like glog,
mysql, etc.) are patched in this codebase.
```suggestion
if grep -q '\-fno-rtti' CMakeLists.txt; then
sed -i 's/-fno-rtti/-frtti/g' CMakeLists.txt
fi
```
##
thirdparty/vars.sh:
##
@@ -512,7 +512,7 @@ JSONCPP_SOURCE="jsoncpp-1.9.5"
JSONCPP_MD5SUM="d6c8c609f2162eff373db62b90a051c7"
# libuuid
-LIBUUID_DOWNLOAD="https://nchc.dl.sourceforge.net/project/libuuid/libuuid-1.0.3.tar.gz";
+LIBUUID_DOWNLOAD="https://downloads.sourceforge.net/project/libuuid/libuuid-1.0.3.tar.gz";
Review Comment:
The libuuid download URL has been changed from a specific mirror
(nchc.dl.sourceforge.net) to the generic SourceForge downloads URL
(downloads.sourceforge.net). While this is a good change for reliability, it is
not mentioned in the PR description or CHANGELOG. Consider either:
1. Adding this to the CHANGELOG as a separate bullet point
2. Mentioning it in the PR description
This helps with transparency and makes it easier to track what changed in
this version.
##
thirdparty/vars.sh:
##
@@ -98,10 +98,10 @@ GTEST_SOURCE=googletest-release-1.12.1
GTEST_MD5SUM="e82199374acdfda3f425331028eb4e2a"
# snappy
-SNAPPY_DOWNLOAD="https://github.com/google/snappy/archive/1.1.8.tar.gz";
-SNAPPY_NAME=snappy-1.1.8.tar.gz
-SNAPPY_SOURCE=snappy-1.1.8
-SNAPPY_MD5SUM="70e48cba7fecf289153d009791c9977f"
+SNAPPY_DOWNLOAD="https://github.com/google/snappy/archive/1.1.10.tar.gz";
+SNAPPY_NAME=snappy-1.1.10.tar.gz
+SNAPPY_SOURCE=snappy-1.1.10
+SNAPPY_MD5SUM="70153395ebe6d72febe2cf2e40026a44"
Review Comment:
The LICENSE-dist.txt file still references the old versions of snappy
(1.1.8) and xxhash (0.8.1) on lines 1497 and 1522 respectively. These should be
updated to reflect the new versions (snappy 1.1.10 and xxhash 0.8.3) being
introduced in this PR.
This file documents the versions of third-party dependencies included in the
distribution and should be kept in sync with actual dependency versions.
##
thirdparty/patches/snappy-1.1.10-sign-compare.patch:
##
@@ -0,0 +1,11 @@
+--- a/snappy.cc
b/snappy.cc
+@@ -1290,7 +1290,7 @@ inline bool Copy64BytesWithPatternExtension(ptrdiff_t
dst, size_t offset) {
+ DeferMemCopy(&deferred_src, &deferred_length, from, len);
+ }
+ } while (ip < ip_limit_min_slop &&
+- (op + deferred_length) < op_limit_min_slop);
++ static_cast(op + deferred_length) <
op_limit_min_slop);
Review Comment:
A new patch file `snappy-1.1.10-sign-compare.patch` has been added, but
there is no corresponding patch application logic in
`thirdparty/download-thirdparty.sh`.
Looking at the existing pattern in download-thirdparty.sh (e.g., lines
252-269 for glog, lines 272-280 for mysql), you need to add a snappy patch
section after the unpacking phase. The section should follow this pattern:
```
# snappy patch
if [[ " ${TP_ARCHIVES[*]} " =~ " SNAPPY " ]]; then
if [[ "${SNAPPY_SOURCE}" == "snappy-1.1.10" ]]; then
cd "${TP_SOURCE_DIR}/${SNAPPY_SOURCE}"
if [[ ! -f "${PATCHED_MARK}" ]]; then
patch -p1 <"${TP_PATCH_DIR}/snappy-1.1.10-sign-compare.patch"
touch "${PATCHED_MARK}"
fi
cd -
fi
echo "Finished patching ${SNAPPY_SOURCE}"
fi
```
Without this, the patch file will not be applied during the build process,
and the sign-compare issue it's meant to fix will persist.
```suggestion
+ reinterpret_cast(op + deferred_length) <
op_limit_min_slop);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
xyla commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3850883715 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [chore](thirdparty) Upgrade snappy and xxhash [doris]
hello-stephen commented on PR #60519: URL: https://github.com/apache/doris/pull/60519#issuecomment-3850881363 Thank you for your contribution to Apache Doris. Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR). Please clearly describe your PR: 1. What problem was fixed (it's best to include specific error reporting information). How it was fixed. 2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be. 3. What features were added. Why was this function added? 4. Which code was refactored and why was this part of the code refactored? 5. Which functions were optimized and what is the difference before and after the optimization? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
