GH-40361: [C++] Make flatbuffers serialization more deterministic

[noamross]

Rationale for this change

This is the start of a PR to address #40361, and in turn #40202, to make metadata in parquet files written by arrow to be identical irrespective of the platform configuration. This is limited, as platform-specific differences in R or Python versions or compression libraries could still result in differences.

What changes are included in this PR?

So far I have only made a partial change to part of the metadata serialization. I need to look at whether other calls to flatbuffers require similar treatment.

Are these changes tested?

Not yet, this is a draft PR

Are there any user-facing changes?

No

• GitHub Issue: [C++] Make Flatbuffers serialization more deterministic #40361

[github-actions]

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

[github-actions]

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

[github-actions]

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

[amoeba]

Thanks @noamross!

@kou can you please approve all workflows to run?

[kou]

Approved!

[github-actions]

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

[pitrou]

As discussed on the GH issue, it would be useful to add a test using output from your machine as a reference.
(also, perhaps open a separate PR with just the test to validate that it would fail on CI)

[amoeba]

Hi @noamross, would you be interested in adding the test described above to this PR? If not, I could take a shot at it.

[noamross]

@amoeba Sorry to open this and abandon it, it just became a very busy month and my C++ is pretty limited. I'd be happy if you would!

[amoeba]

No problem at all @noamross, it's awesome you filed a PR for the issue and it's very much appreciated. I'll work on adding and testing the test.

[noamross]

Thanks! I expect the files attached to #40202 (comment) could serve for a test of whether a generated file is identical to a reference from a different system, but I note that they would change with the R version being used.

[amoeba]

I put up a draft PR with a test to exercise this and hopefully watch it fail on CI at #41018. It fails on my amd64 linux machine so I think it's close to what we need here. I'll watch CI on that PR and report back.

[amoeba]

@github-actions crossbow submit -g cpp

[github-actions]

Only contributors can submit requests to this bot. Please ask someone from the community for help with getting the first commit in.
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/8668333493

[amoeba]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: 9117945

Submitted crossbow builds: ursacomputing/crossbow @ actions-e9bd83c5da

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-14

[amoeba]

Things look good locally (testing on macOS-aarch64-clang and Linux-amd64-gcc) but I see an issue with my includes in a few of the above jobs. I'll fix those soon.

[amoeba]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: c3af79d

Submitted crossbow builds: ursacomputing/crossbow @ actions-08f54365f0

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-14

[amoeba]

I've pushed up a fix to address the includes issue and the crossbow jobs looked clear (failures are unrelated and the test we expect to pass does indeed pass).

I've marked this as ready for review. @pitrou do you have time to review this? cc @kou

[amoeba]

Thanks @pitrou for the review. I rebased from apache/main, accepted your three changes as-is, and added one extra change I caught while dealing with conflicts on rebase, 4bd0339. In 4c8eff2 @kou tweaked the CMakeList and I think this PR should make the same change as was made there.

PS: After pushing I found the ipc/CMakeLists.txt needed formatting, so that was added here too.

[amoeba]

Parquet tests were failing all over on CI and due to needing to bump submodules so I pushed 4ebe53b so we could see CI.

[pitrou]

@github-actions crossbow submit -g cpp

[pitrou]

It seems there are still spurious parquet-testing updates in this PR?

[github-actions]

Revision: 4ebe53b

Submitted crossbow builds: ursacomputing/crossbow @ actions-d1985bc136

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-14

[mapleFU]

By the way, can you switch the edit of parquet-testing back?

[amoeba]

Thanks all. The submodule change to help CI pass has been reverted in this PR. I'm merging this now.

[conbench-apache-arrow]

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

There were no benchmark performance regressions. 🎉

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