Skip to content

GH-39973: [C++][CI] Disable debug memory pool for ASAN and Valgrind#39975

Merged
pitrou merged 10 commits intoapache:mainfrom
zanmato1984:asan
Feb 8, 2024
Merged

GH-39973: [C++][CI] Disable debug memory pool for ASAN and Valgrind#39975
pitrou merged 10 commits intoapache:mainfrom
zanmato1984:asan

Conversation

@zanmato1984
Copy link
Copy Markdown
Contributor

@zanmato1984 zanmato1984 commented Feb 7, 2024

Rationale for this change

Disable debug memory pool for ASAN and Valgrind so that they can detect more subtle memory issues regarding to buffer tail bytes.

What changes are included in this PR?

  1. Add a none option to debug memory pool env var to make other things slightly easier.
  2. Change *_test.sh scripts to conditionally set debug memory pool env var.
  3. Top-level docker compose change to pass none to debug memory pool env var for ASAN and Valgrind.

Are these changes tested?

The CI should cover it well.

Are there any user-facing changes?

No.

@zanmato1984
Copy link
Copy Markdown
Contributor Author

cc @pitrou

@zanmato1984
Copy link
Copy Markdown
Contributor Author

If things go well, we should be seeing the ASAN failure as described in #39976 somewhere in CI job. This is expected as we are strengthening the ASAN check so hidden legacy failures are exposed.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 7, 2024

@github-actions crossbow submit -g cpp

@apache apache deleted a comment from zanmato1984 Feb 7, 2024
@apache apache deleted a comment from github-actions bot Feb 7, 2024
@apache apache deleted a comment from github-actions bot Feb 7, 2024
@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 7, 2024

I've cleaned up the failed Crossbow requests :-)

@github-actions

This comment was marked as outdated.

@kou kou changed the title GH-39973: [C++] [CI] Disable debug memory pool for ASAN and Valgrind GH-39973: [C++][CI] Disable debug memory pool for ASAN and Valgrind Feb 8, 2024
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Feb 8, 2024
@zanmato1984
Copy link
Copy Markdown
Contributor Author

The legacy ASAN failure will be gone once #39994 goes in.

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 8, 2024
@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 8, 2024

I've rebased, hopefully CI wil be green now.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 8, 2024

@github-actions crossbow submit -g cpp

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 8, 2024

Revision: b9ff2f3

Submitted crossbow builds: ursacomputing/crossbow @ actions-1f71ea13ea

Task Status
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind Azure
test-cuda-cpp GitHub Actions
test-debian-11-cpp-amd64 GitHub Actions
test-debian-11-cpp-i386 GitHub Actions
test-fedora-38-cpp GitHub Actions
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-no-threading GitHub Actions

@zanmato1984
Copy link
Copy Markdown
Contributor Author

I've rebased, hopefully CI wil be green now.

Thank you for rebasing.

Praying for CI now.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 8, 2024

You can stop praying now :-)

@pitrou pitrou merged commit a946214 into apache:main Feb 8, 2024
@pitrou pitrou removed the awaiting change review Awaiting change review label Feb 8, 2024
@conbench-apache-arrow
Copy link
Copy Markdown

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

There were 4 benchmark results indicating a performance regression:

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.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…rind (apache#39975)

### Rationale for this change

Disable debug memory pool for ASAN and Valgrind so that they can detect more subtle memory issues regarding to buffer tail bytes.

### What changes are included in this PR?

1. Add a `none` option to debug memory pool env var to make other things slightly easier.
2. Change `*_test.sh` scripts to conditionally set debug memory pool env var.
3. Top-level docker compose change to pass none to debug memory pool env var for ASAN and Valgrind.

### Are these changes tested?

The CI should cover it well.

### Are there any user-facing changes?

No.

* Closes: apache#39973

Authored-by: Ruoxi Sun <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
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.

[C++][CI] Debug memory pool interferes with ASAN check

3 participants