Skip to content

Conversation

@Vishwanatha-HD
Copy link
Contributor

@Vishwanatha-HD Vishwanatha-HD commented Nov 21, 2025

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

@github-actions
Copy link

⚠️ GitHub issue #48220 has been automatically assigned in GitHub to PR creator.

@Vishwanatha-HD Vishwanatha-HD force-pushed the fixArrowBufArrowMiscIssues branch from 64d5429 to 6828f2c Compare November 22, 2025 05:08
@kou kou changed the title GH-48220 Fix arrow-buffer & arrow-misc testcase failures to enable Pa… GH-48220: [C++][Parquet] Fix arrow-buffer & arrow-misc testcase failures to enable Parquet DB support on s390x Nov 22, 2025
@kou
Copy link
Member

kou commented Nov 22, 2025

Could you update the PR title and description?

  • This doesn't fix failures. This just skips failed tests.
  • This isn't for Parquet. This is for Arrow.

@k8ika0s
Copy link

k8ika0s commented Nov 23, 2025

@Vishwanatha-HD

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 OutOfMemory, which can make the test suite look broken even though the actual Arrow logic is fine.

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 1 << 48 and only skipping when the allocator backend is known to abort, which keeps the OOM behavior testable without triggering allocator issues.

@Vishwanatha-HD Vishwanatha-HD changed the title GH-48220: [C++][Parquet] Fix arrow-buffer & arrow-misc testcase failures to enable Parquet DB support on s390x GH-48220: [C++] Fix arrow-buffer & arrow-misc testcase failures on s390x Nov 26, 2025
@github-actions
Copy link

⚠️ GitHub issue #48220 has no components, please add labels for components.

@Vishwanatha-HD Vishwanatha-HD force-pushed the fixArrowBufArrowMiscIssues branch from 6828f2c to c837d07 Compare November 29, 2025 13:12
// 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
Copy link
Member

Choose a reason for hiding this comment

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

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?


TYPED_TEST_P(TestMemoryPool, MemoryTracking) { this->TestMemoryTracking(); }

// Skip this test on big-endian architectures (e.g., s390x)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Nov 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants