GH-49465: [CI][C++] Fix Abseil hang in arrow-flight-test on ODBC Windows#50085
GH-49465: [CI][C++] Fix Abseil hang in arrow-flight-test on ODBC Windows#50085tadeja wants to merge 7 commits into
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR adds a temporary CI-only workaround for intermittent hangs at process exit in arrow-flight-test on the Windows ODBC (MSVC) job, by rebuilding gRPC and Arrow with GPR_DISABLE_ABSEIL_SYNC to avoid Abseil sync primitives on Windows.
Changes:
- Add a custom vcpkg overlay triplet (
x64-windows-no-absl-sync) that definesGPR_DISABLE_ABSEIL_SYNCduring dependency builds. - Update the Windows ODBC CI job to use the new vcpkg triplet and to compile Arrow with the same macro to keep ABI consistent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ci/vcpkg/overlay-triplets-no-absl-sync/x64-windows-no-absl-sync.cmake |
New vcpkg triplet to rebuild packages (notably gRPC) with GPR_DISABLE_ABSEIL_SYNC. |
.github/workflows/cpp_extra.yml |
Switch Windows ODBC job to the new triplet and add Arrow compiler flags for the workaround. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # https://github.com/abseil/abseil-cpp/issues/1877 | ||
| # Build Arrow with GPR_DISABLE_ABSEIL_SYNC so grpcpp's Mutex ABI | ||
| # matches the gRPC rebuilt by the overlay triplet. Remove once fixed upstream. | ||
| ARROW_CXXFLAGS: -DGPR_DISABLE_ABSEIL_SYNC |
| # GH-49465: custom triplet that rebuilds gRPC with GPR_DISABLE_ABSEIL_SYNC | ||
| # (native sync instead of absl::Mutex). See ci/vcpkg/overlay-triplets-no-absl-sync. | ||
| VCPKG_DEFAULT_TRIPLET: x64-windows-no-absl-sync | ||
| VCPKG_OVERLAY_TRIPLETS: ${{ github.workspace }}/ci/vcpkg/overlay-triplets-no-absl-sync |
There was a problem hiding this comment.
How about using ci/vcpkg/ not ci/vcpkg/overlay-triplets-no-absl-sync/?
We already have many triplet files in ci/vcpkg/.
There was a problem hiding this comment.
Done, moved to ci/vcpkg/
| VCPKG_DEFAULT_TRIPLET: x64-windows | ||
| # GH-49465: custom triplet that rebuilds gRPC with GPR_DISABLE_ABSEIL_SYNC | ||
| # (native sync instead of absl::Mutex). See ci/vcpkg/overlay-triplets-no-absl-sync. | ||
| VCPKG_DEFAULT_TRIPLET: x64-windows-no-absl-sync |
There was a problem hiding this comment.
We already have amd64-windows-static-md-{debug,release}.cmake in ci/vcpkg/.
How about aligning naming convention with them?
There was a problem hiding this comment.
Done, renamed to amd64-windows-no-absl-sync-release
github-actions
Bot
added
awaiting changes
github-actions
Bot
added
awaiting change review
Rationale for this change
arrow-flight-testpasses all its subtests then intermittently hangs on exit and ctest kills it at the 300s timeout. The last thread was parked in Abseil's Windows sync primitiveWin32Waiterduring static destruction inside statically-linked gRPC. See upstream grpc/grpc#39321 and abseil/abseil-cpp#1877What changes are included in this PR?
For the ODBC Windows CI job: build gRPC (via the
amd64-windows-no-absl-sync-releasevcpkg overlay triplet inci/vcpkg) and Arrow (viaARROW_CXXFLAGS) withGPR_DISABLE_ABSEIL_SYNC, so both use native Windows sync instead ofabsl::Mutex. Remove once fixed upstream.Are these changes tested?
Yes, on fork CI the ODBC Windows tests were re-run with
ctest --repeat until-fail:10(temporary, not part of this PR) - All tests exited successfully thereAre there any user-facing changes?
No.