bazel/avro: make avrogen header include guard deterministic#30487
Closed
travisdowns wants to merge 1 commit into
Closed
bazel/avro: make avrogen header include guard deterministic#30487travisdowns wants to merge 1 commit into
travisdowns wants to merge 1 commit into
Conversation
The generated header's include guard was suffixed with the output of a `boost::mt19937` seeded from `::time(nullptr)` (`avrogencpp.cc:796`), producing a different guard on every avrogen invocation: #ifndef BAZEL_OUT_..._MANIFEST_FILE_AVROGEN_H_3350718792_H #ifndef BAZEL_OUT_..._MANIFEST_FILE_AVROGEN_H_2362587291_H That made every consumer of the generated header (e.g. manifest_list_avro.cc) see a different input digest on each build, so bazel's remote cache was broken for the entire downstream dependency chain even on byte-identical schemas. `headerFile_` is already a unique-per-output path; dropping the random suffix leaves a stable, still-unique guard. Identified via two-output-base hermetic builds of `//tools:redpanda_package_for_pgo` as one of the two roots that infect that target. Tracks CORE-16317.
Member
Author
|
Closing in favor of bumping the pinned |
Collaborator
Retry command for Build#84489please wait until all jobs are finished before running the slash command |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
avrogencpptool (viabazel/thirdparty/avrogen-stable-include-guard.patch) so the generated header's include guard no longer carries a time-seeded random suffix.boost::mt19937random number fromlang/c++/impl/avrogencpp.cc:796— the canonicalizedheaderFile_is already path-unique.Why
The current avrogen produces a guard like
BAZEL_OUT_..._MANIFEST_FILE_AVROGEN_H_<random>_Hwhere<random>comes from aboost::mt19937seeded with::time(nullptr). Two invocations on the same schema produce headers with different bytes, so every consumer of the generated header (manifest_list_avro.cc) sees a different input digest on every build. That blows the bazel remote cache for the entire downstream chain —iceberglib →redpandabinary →//tools:redpanda_package_for_pgo— even when the schema is byte-identical.Identified via the
beptool walkbackanalysis of two-output-base builds of//tools:redpanda_package_for_pgo: it's one of only two persistent root causes (the other beingmake_tool, CORE-16315) that infect that target across overnight runs. Fixing this is half of the work to make the PGO instrument tar reproducible, which would let the ~1956 PGO-optimize compile actions in the overnight builds (84428–84432) actually hit the remote cache instead of re-executing locally on every run.Tracks CORE-16317.
Validation
Before this patch the two hashes differed. After:
New include guard:
BAZEL_OUT_K8_FASTBUILD_BIN_SRC_V_ICEBERG_MANIFEST_FILE_AVROGEN_H_H(deterministic; the trailing_H_His cosmetic — could be simplified to justhif reviewers prefer).Upstream
I'll open a corresponding PR against
apache/avroso other users of avrogen with hermetic build systems (Bazel, Nix, etc.) get the fix.Test plan
//src/v/iceberg:manifest_file_genruleproduce byte-identical headers//tools:redpanda_package_for_pgotar diverges only on the remainingmake_toolroot after this lands