GH-48220: [C++] Fix arrow-buffer & arrow-misc testcase failures on s390x#48221
GH-48220: [C++] Fix arrow-buffer & arrow-misc testcase failures on s390x#48221Vishwanatha-HD wants to merge 1 commit into
Conversation
|
|
64d5429 to
6828f2c
Compare
|
Could you update the PR title and description?
|
|
Thanks for taking a look at the buffer and OOM paths — these tests always feel a little fragile, and every platform seems to expose a different edge case. One thing I’ve run into on s390x is that some of the allocators (mimalloc in particular) don’t fail gracefully on the “impossible size” cases. They sometimes hit fatal paths before Arrow ever gets a chance to raise I noticed this version skips the test entirely on big-endian builds. That definitely avoids the allocator crash, though it does mean BE never exercises the OOM branch at all. In my own testing I’ve had some luck clamping the huge allocation down to something like |
|
|
6828f2c to
c837d07
Compare
| // This test doesn't play nice with AddressSanitizer | ||
| #ifndef ADDRESS_SANITIZER | ||
| // Skip this test on big-endian architectures (e.g., s390x) | ||
| #if !defined(ADDRESS_SANITIZER) && ARROW_LITTLE_ENDIAN |
There was a problem hiding this comment.
Could you please write why we need to skip this test shortly in this comment? In addition, could you please include the link to this PR and include the detail in the description of the PR?
There was a problem hiding this comment.
Same concern. It's not obvious why this test won't work on big-endian so a brief explanation would help.
There was a problem hiding this comment.
These are my observations:
- The "mimalloc" allocation request enters mimalloc's large-object allocation path.
- Mimalloc attempts to calculate reservation sizes, alignment, segments, and OS mappings.
- On s390x systems with huge virtual address spaces and Linux overcommit enabled, the allocation may not fail immediately.
- Mimalloc is spending a significant amount of time trying to satisfy or validate the request before ultimately failing.
- The testcase "hangs" because the control never returning to Arrow.
|
|
||
| TYPED_TEST_P(TestMemoryPool, MemoryTracking) { this->TestMemoryTracking(); } | ||
|
|
||
| // Skip this test on big-endian architectures (e.g., s390x) |
There was a problem hiding this comment.
These are my observations:
- The "mimalloc" allocation request enters mimalloc's large-object allocation path.
- Mimalloc attempts to calculate reservation sizes, alignment, segments, and OS mappings.
- On s390x systems with huge virtual address spaces and Linux overcommit enabled, the allocation may not fail immediately.
- Mimalloc is spending a significant amount of time trying to satisfy or validate the request before ultimately failing.
- The testcase "hangs" because the control never returning to Arrow.
|
@pitrou @kou @kiszk @zanmato1984 |
Vishwanatha-HD
left a comment
There was a problem hiding this comment.
I have tried answering your questions. Please take a look. Thanks.
|
|
||
| TYPED_TEST_P(TestMemoryPool, MemoryTracking) { this->TestMemoryTracking(); } | ||
|
|
||
| // Skip this test on big-endian architectures (e.g., s390x) |
There was a problem hiding this comment.
These are my observations:
- The "mimalloc" allocation request enters mimalloc's large-object allocation path.
- Mimalloc attempts to calculate reservation sizes, alignment, segments, and OS mappings.
- On s390x systems with huge virtual address spaces and Linux overcommit enabled, the allocation may not fail immediately.
- Mimalloc is spending a significant amount of time trying to satisfy or validate the request before ultimately failing.
- The testcase "hangs" because the control never returning to Arrow.
| // This test doesn't play nice with AddressSanitizer | ||
| #ifndef ADDRESS_SANITIZER | ||
| // Skip this test on big-endian architectures (e.g., s390x) | ||
| #if !defined(ADDRESS_SANITIZER) && ARROW_LITTLE_ENDIAN |
There was a problem hiding this comment.
These are my observations:
- The "mimalloc" allocation request enters mimalloc's large-object allocation path.
- Mimalloc attempts to calculate reservation sizes, alignment, segments, and OS mappings.
- On s390x systems with huge virtual address spaces and Linux overcommit enabled, the allocation may not fail immediately.
- Mimalloc is spending a significant amount of time trying to satisfy or validate the request before ultimately failing.
- The testcase "hangs" because the control never returning to Arrow.
|
@Vishwanatha-HD Can you try rebasing your PR? We are using a newer mimalloc version now and your test skips could have become unnecessary. |
…ble Parquet DB support on s390x
c837d07 to
a77d1ea
Compare
|
Hi @pitrou.. I did the rebase of my PR.. ctest -R "buffer|memory_pool" -VV 22: Test command: arrow/cpp/build-support/run-test.sh " To confirm the working, I did set the "ARROW_DEFAULT_MEMORY_POOL" to "system" and ran the test again.. I see that all the tests passes in this case.. ctest -R "buffer|memory_pool" -VV 22: Test command: /home/vishwa/golang/arrowGitRepo/arrow/cpp/build-support/run-test.sh "/home/vishwa/golang/arrowGitRepo/arrow/cpp/build" "test" "/home/vishwa/golang/arrowGitRepo/arrow/cpp/build/debug//arrow-buffer-test" 56: Test command: /home/vishwa/golang/arrowGitRepo/arrow/cpp/build-support/run-test.sh "/home/vishwa/golang/arrowGitRepo/arrow/cpp/build" "test" "/home/vishwa/golang/arrowGitRepo/arrow/cpp/build/debug//arrow-io-buffered-test" The following tests passed: 100% tests passed, 0 tests failed out of 2 Label Time Summary: Total Test time (real) = 0.15 sec |
Rationale for this change
This PR is intended to enable Parquet DB support on Big-endian (s390x) systems. The fix in this PR fixes the "arrow-buffer-test" & "arrow-misc-test" failures on s390x.
What changes are included in this PR?
The fix includes changes to following files:
cpp/src/arrow/buffer_test.cc
cpp/src/arrow/memory_pool_test.cc
Are these changes tested?
Yes. The changes are tested on s390x arch to make sure things are working fine. The fix is also tested on x86 arch, to make sure there is no new regression introduced.
Are there any user-facing changes?
No.
GitHub main Issue link: #48151