Skip to content

Bug/update libcudf to handle arrow12 changes - #13794

Merged
rapids-bot[bot] merged 7 commits into
NVIDIA:branch-23.10from
robertmaynard:bug/update_libcudf_to_handle_arrow12_changes
Aug 2, 2023
Merged

Bug/update libcudf to handle arrow12 changes#13794
rapids-bot[bot] merged 7 commits into
NVIDIA:branch-23.10from
robertmaynard:bug/update_libcudf_to_handle_arrow12_changes

Conversation

@robertmaynard

Copy link
Copy Markdown
Contributor

Description

Contiuation of #13790 as more changes are needed to support Arrow 12 builds from source ( both static and shared ). This fixes issues when building against Arrow with S3 enabled, and corrects missing acero targets.

NVIDIA/cudf-spark-jni#1306

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

On application shutdown libcudf needs to call `EnsureS3Finalized`
before exit. This is tricky since the arrow might not be compiled
with S3 support enabled.

So use dlopen to check for the symbol existence, and call when
it does exist. Only do this in the S3 tests for now as we figure
out a longer term solution.
@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Aug 1, 2023
@robertmaynard
robertmaynard requested review from a team as code owners August 1, 2023 18:22
@github-actions github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Aug 1, 2023
@robertmaynard

Copy link
Copy Markdown
Contributor Author

apache/arrow#36974 is the upstream issue which covers why we need to use dlopen + dlsym to call the S3 finialize function.

Comment thread cpp/tests/io/arrow_io_source_test.cpp Outdated
Comment thread cpp/cmake/thirdparty/get_arrow.cmake Outdated
Comment thread cpp/tests/io/arrow_io_source_test.cpp
@gerashegalov

Copy link
Copy Markdown
Contributor

spark-rapids-jni runs into

[INFO]      [exec] CMake Error in CMakeLists.txt:
[INFO]      [exec]   export called with target "arrow_dataset_static" which requires target
[INFO]      [exec]   "arrow_acero_static" that is not in any export set. 

with the current state of the PR as the next issue

robertmaynard and others added 3 commits August 1, 2023 14:35
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
Co-authored-by: Bradley Dice <bdice@bradleydice.com>

@davidwendt davidwendt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving C++ changes.

@gerashegalov gerashegalov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, spark-rapids-jni build is working with this PR. Thanks!

@robertmaynard
robertmaynard requested a review from bdice August 2, 2023 13:32
@robertmaynard

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit d526530 into NVIDIA:branch-23.10 Aug 2, 2023
@robertmaynard
robertmaynard deleted the bug/update_libcudf_to_handle_arrow12_changes branch August 7, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants