Skip to content

Conversation

@pitrou
Copy link
Member

@pitrou pitrou commented Aug 8, 2024

Install the clang-rt libraries that are necessary to link Thread Sanitizer-enabled binaries. Also fix use of deprecated BufferReader constructor in some tests, so that compilation with CLang 18 succeeds.

Note that the C++ test suite still fails on Flight tests, as tracked in GH-36552.

… Ubuntu 24.04

Install the clang-rt libraries that are necessary to link Thread Sanitizer-enabled binaries.

Note that the C++ test suite then fails on Flight tests, as tracked in apacheGH-36552.
@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

cc @kou

@github-actions github-actions bot added the awaiting review Awaiting review label Aug 8, 2024
@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

@github-actions crossbow submit ubuntucpp*

@github-actions
Copy link

github-actions bot commented Aug 8, 2024

Revision: 0019b58

Submitted crossbow builds: ursacomputing/crossbow @ actions-d48e76f193

Task Status
test-ubuntu-20.04-cpp GitHub Actions
test-ubuntu-20.04-cpp-bundled GitHub Actions
test-ubuntu-20.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-20.04-cpp-thread-sanitizer GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions

@kou
Copy link
Member

kou commented Aug 8, 2024

Do we need this too?

diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml
index 6e1f7609a9..50ef83905d 100644
--- a/dev/tasks/tasks.yml
+++ b/dev/tasks/tasks.yml
@@ -1168,16 +1168,16 @@ tasks:
       flags: "-e ARROW_ENABLE_THREADING=OFF"
       image: ubuntu-cpp
 
-  test-ubuntu-20.04-cpp-thread-sanitizer:
+  test-ubuntu-24.04-cpp-thread-sanitizer:
     ci: github
     template: docker-tests/github.linux.yml
     params:
       env:
         # clang-tools and llvm version need to be synchronized so as
         # to have the right llvm-symbolizer version
-        CLANG_TOOLS: 11
-        LLVM: 11
-        UBUNTU: 20.04
+        CLANG_TOOLS: 18
+        LLVM: 18
+        UBUNTU: 24.04
       image: ubuntu-cpp-thread-sanitizer
 
   test-ubuntu-20.04-cpp-minimal-with-formats:

@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

We can't because of #36552

@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

Wait, let me try with those LLVM/CLang versions

@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

Indeed, the Flight tests still fail with those versions. However, CLang 18 also surfaces some minor C++ issues that I can fix here.

@pitrou
Copy link
Member Author

pitrou commented Aug 8, 2024

@kou Do these changes look ok?

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

I haven't tested this on local but it will work.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Aug 9, 2024
@pitrou pitrou merged commit 7c8909a into apache:main Aug 14, 2024
@pitrou pitrou removed the awaiting merge Awaiting merge label Aug 14, 2024
@pitrou pitrou deleted the minor-ubuntu-24.04-tsan branch August 14, 2024 12:27
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 7c8909a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

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.

2 participants