GH-44607: [C++][Dev] Update bundled Thrift, update mirrors to use CDN #44685
Conversation
|
@github-actions crossbow submit test-r-linux-as-cran |
|
Revision: 7173f31 Submitted crossbow builds: ursacomputing/crossbow @ actions-c90acec2ee
|
|
@github-actions crossbow submit -g cpp |
|
Revision: 77d7c31 Submitted crossbow builds: ursacomputing/crossbow @ actions-da415e2d2d
|
|
@github-actions crossbow submit -g cpp |
|
Revision: 2bbe2aa Submitted crossbow builds: ursacomputing/crossbow @ actions-4c1b909295
|
|
Windows MSVC is failing with :
rhub/ubuntu-gcc-12 is failing with:
|
|
The two failing builds were both bundling Boost and Thrift and I was able to reproduce the failure locally. From looking around, it looks like you have to upgrade to Boost 1.86.0 if you use Thrift 0.21.0. To test that hypothesis, I lowered Thrift down to 0.20.0 in 4886ad7 to test. I think we can get away with bumping Boost to 1.86.0 and Thrift to 0.21.0 in this PR so I'll try that once CI finishes. |
|
With Boost 1.86.0 and Thrift 0.21.0, the MSVC build fails with: |
|
It looks like our build may be conflicting with a change in Thrift that appeared in 0.21.0: |
|
@github-actions crossbow submit -g cpp |
|
Revision: 88601bc Submitted crossbow builds: ursacomputing/crossbow @ actions-46a48b959a
|
|
@assignUser this is ready for a review. Three of the four failed crossbow jobs looks like test flakiness but another set of eyes would be good. The Edit: On the last point, I think maybe we can hold off on updating the ORC mirror URLs. I looked closer and it looks like the runner VM had a more fundamental networking issue at the time so maybe we can just keep an eye on this. |
cpp/thirdparty/versions.txt
Outdated
| "ARROW_BOOST_URL boost-${ARROW_BOOST_BUILD_VERSION}.tar.gz https://apache.jfrog.io/artifactory/arrow/thirdparty/7.0.0/boost_${ARROW_BOOST_BUILD_VERSION//./_}.tar.gz" | ||
| "ARROW_BOOST_URL boost-${ARROW_BOOST_BUILD_VERSION}.tar.gz https://archives.boost.io/release/1.86.0/source/boost_${ARROW_BOOST_BUILD_VERSION//./_}.tar.gz" |
There was a problem hiding this comment.
We use a trimmed down version of boost (10mb vs 142mb) to speed up downloads. Could you create the new archive and upload it to the artifactory (as a committer you can do that) and use that version here?
There was a problem hiding this comment.
Will do. I just tried to run the script and am getting failures on the bootstrap/b2 step. I'll take a look later today.
There was a problem hiding this comment.
I didn't have any luck and it appears to be caused by Boost 1.86.0 releasing with a broken bcp. See comment and really the whole thread at boostorg/bcp#18 (comment).
I think we could probably update the trim-boost script to work around this but we could also just switch back to Thrift 0.20.0 and Boost 1.81.0 until Boost 1.87.0.
github-actions
bot
added
awaiting committer review
|
Due to the bcp issue with boost 1.86.0, I bumped Thrift down to 0.20.0 and Boost back to 1.81.0. We can update to Thrift 0.21.0 once Boost 1.87.0 is out (and we can confirm the included bcp works). |
|
CI passed and PR description updated. @assignUser I think we can merge this without re-running all of the crossbow jobs and just be mindful during the 18.1.0 release process to look out for any issues. |
| @@ -1775,6 +1763,7 @@ macro(build_thrift) | |||
| set(THRIFT_LIB_SUFFIX "md") | |||
| list(APPEND THRIFT_CMAKE_ARGS "-DWITH_MT=OFF") | |||
| endif() | |||
| # NOTE(amoeba): When you bump Thrift to >=0.21.0, change bin to lib | |||
|
Merged, thanks. |
…#44685) ### Rationale for this change Builds with bundled Thrift could fail because of permanently offline download mirror URLs. ASF now provides [a CDN](https://dlcdn.apache.org/). ### What changes are included in this PR? 1. Updates the pinned version for bundled Thrift from 0.16.0 to 0.20.0. We couldn't just update the URLs because the ASF CDN only hosts the last two versions of Thrift (0.20, 0.21.0). We didn't upgrade to Thrift 0.21.0 in this PR because it requires Boost 1.86.0 and we found incompatibilities with Boost 1.86.0 and `cpp/build-support/trim-boost.sh` which we use to publish a trimmed-down distribution of Boost for our build system to use. 2. Removes the set of URLs we were using to download Thrift in favor of just using the main CDN URL. We'll see how this works in practice. ### Are these changes tested? Yes. I configured a build with `...-DThrift_SOURCE=BUNDLED...` and ran the full C++ test suite. I think further testing can be done in CI. ### Are there any user-facing changes? No. Closes #44607 * GitHub Issue: #44607 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Bryce Mecum <[email protected]>
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d7e982c. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Builds with bundled Thrift could fail because of permanently offline download mirror URLs. ASF now provides a CDN.
What changes are included in this PR?
cpp/build-support/trim-boost.shwhich we use to publish a trimmed-down distribution of Boost for our build system to use.Are these changes tested?
Yes. I configured a build with
...-DThrift_SOURCE=BUNDLED...and ran the full C++ test suite. I think further testing can be done in CI.Are there any user-facing changes?
No.
Closes #44607