Skip to content

[fix](be) Make the BE unit tests build and run on macOS - #68520

Merged
morningman merged 2 commits into
apache:masterfrom
morningman:fix-macos-be-ut-compile
Sep 28, 2026
Merged

morningman merged 2 commits into
apache:masterfrom
morningman:fix-macos-be-ut-compile

Conversation

@morningman

@morningman morningman commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

Context. run-be-ut.sh is the only entry point for the BE unit tests, and on macOS it is also the only way to run them: the BE UT (macOS) workflow runs build.sh --be, which configures with -DMAKE_TEST=OFF, so nothing under be/test is compiled there (the workflow says so in its own comment at .github/workflows/be-ut-mac.yml:123). On a clean worktree the script therefore fails twice before a single test executes, and then stops seven more times while compiling be/test.

1. The problem, and what it cost

The script never initializes two of the submodules it needs. run-be-ut.sh initializes three submodules (contrib/datasketches-cpp, contrib/apache-orc, contrib/clucene); build.sh initializes five, and the two it adds — contrib/faiss and contrib/openblas — are required before a single file is compiled. be/CMakeLists.txt:1038 adds be/src/storage/index/ann unconditionally, and that directory's cmake-protect/CMakeLists.txt:48,66 calls add_subdirectory() on both. On a fresh clone or a new git worktree the configure step therefore ends with

CMake Error at src/storage/index/ann/cmake-protect/CMakeLists.txt:48 (add_subdirectory):
  The source directory .../contrib/openblas does not contain a CMakeLists.txt file.
CMake Error at src/storage/index/ann/cmake-protect/CMakeLists.txt:66 (add_subdirectory):
  The source directory .../contrib/faiss does not contain a CMakeLists.txt file.

Running build.sh once first hides it, which is why it goes unnoticed.

Seven be/test translation units do not compile on macOS. The Linux BE UT build is green for all seven. run-be-ut.sh invokes ninja without -k, so the developer fixes one file, waits out another full compile, and meets the next one — seven round trips, each of them tens of minutes, for a task whose whole point is to run a test.

# TU Error Root cause
1 exprs/function/function_map_inner_product_test.cpp 'faiss/impl/platform_macros.h' file not found The APPLE branch of be/test/CMakeLists.txt:170 links vector_search_test as -Wl,-force_load,$<TARGET_FILE:vector_search_test>: a file path, which carries no usage requirements. Every other platform links -Wl,--whole-archive vector_search_test, which is a target in the link list and does propagate them.
2 exprs/function/function_format_round_test.cpp ambiguous conversion for functional-style cast from 'long' to 'Decimal64' (11 sites) Decimal64 is Decimal<int64_t>, so on Darwin the constructor parameter is long long while these literals are long; every integral and floating constructor is then an equally ranked conversion away.
3 storage/index/snii/bench/bkd_native_vs_clucene_bench_test.cpp six -Werror,-Wformat uint64_t is unsigned long long on Darwin and unsigned long on Linux, so %lu / %ld is right on one and wrong on the other.
4 storage/index/snii/bench/snii_vs_v3_benchmark_test.cpp no member named 'posix_fadvise', mincore argument type posix_fadvise / POSIX_FADV_DONTNEED do not exist on Darwin; mincore(2) takes char* there and unsigned char* on Linux.
5 storage/index/snii/bkd/bkd_corruption_test.cpp deduced conflicting types ('size_type' vs 'uint64_t') An initializer list mixing payload.size() (size_t) with uint64_t literals.
6 storage/index/snii/writer/spimi_locality_bench_test.cpp libc++ no matching function for call to object of type 'key_equal' The transparent equality functor defines one argument order only. libc++ calls key_equal as (key, lookup key), libstdc++ as (lookup key, key).
7 storage/segment/hierarchical_data_iterator_test.cpp three no matching function for call to 'min' kRows is size_t and current_ordinal is ordinal_t (uint64_t), which are distinct types on Darwin, so std::min cannot deduce a common type.

None of the seven is related to a local change; a clean master reproduces all of them. #2 arrived on 2026-09-24 with #68397 ([fix](be) Preserve decimal precision in format_round, by TengJianPing), which is the commit that added function_format_round_test.cpp — after the last full macOS sweep. So this class of breakage is still being produced: "green on Linux, red on macOS" is invisible to every CI job we have, and the two obstacles above make the one manual path that would find it expensive enough that nobody walks it.

2. What this PR does, and why it helps

  • run-be-ut.sh: the update_submodule list gains contrib/openblas and contrib/faiss, with the same archive URLs build.sh:941 already uses, plus a comment that the list has to stay in sync with build.sh.
  • be/test/CMakeLists.txt: the APPLE branch links ann_index in addition to the force_load flag. ann_index is the target that exports the faiss include directories PUBLIC, so the APPLE link line now feeds the same compile and link inputs as every other platform, instead of being a macOS special case.
  • Six test files: the smallest change that removes the platform difference, each spelled the way the file's neighbours already spell it — PRIu64 / PRId64 (core/decimal12.h, util/histogram.cpp), int64_t(...) around the literals (storage/key_coder_test.cpp:392, runtime/jsonb_document_test.cpp:433), #if defined(POSIX_FADV_DONTNEED) around the call.

Linux behaviour is unchanged: every edit is either inside an APPLE-only path or a strictly more portable spelling of the same value. On macOS run-be-ut.sh becomes a usable test line for the first time.

One cost worth naming: because the APPLE link line now propagates include directories, the compile command of every be/test TU changes once, so the first macOS build after this lands rebuilds be/test (about 1230 TUs). Later builds are incremental again.

Branch pick. #68397 was never picked to branch-4.1, so function_format_round_test.cpp does not exist there. Of the seven files, only storage/segment/hierarchical_data_iterator_test.cpp exists on branch-4.1, with the same three std::min(*rows, ROWS - current_ordinal) sites and the same types (ROWS is size_t, ordinal_t is uint64_t), so it fails to compile on macOS there too; that branch also has no APPLE force_load branch in be/test/CMakeLists.txt, so the faiss part is master-only. If 4.1 is meant to run BE UT on macOS, the pick is that one file's hunk.

3. The classes, and how they call each other

  • run-be-ut.sh (changed): update_submodule() calls git submodule update --init --recursive <path> and falls back to the branch tarball; the two added lines are identical in shape to the three that were already there.
  • be/test/CMakeLists.txt (changed): the APPLE branch of the doris_be_test link step.
  • be/src/storage/index/ann/cmake-protect/CMakeLists.txt (unchanged, the reason the submodules are mandatory): add_subdirectory() on contrib/openblas and contrib/faiss, reached unconditionally from be/CMakeLists.txt:1038.
  • ann_index (unchanged target in be/src/storage/index/ann): links faiss PUBLIC, so its INTERFACE_INCLUDE_DIRECTORIES carry the faiss include path.
  • vector_search_test (unchanged target in be/test/storage/index/ann): links ann_index PUBLIC; on APPLE it reaches doris_be_test as forced-in objects rather than as a target.
  • The seven test TUs: unchanged in behaviour, three of them compiled in a portable spelling.
run-be-ut.sh
  |- update_submodule contrib/{datasketches-cpp, apache-orc, clucene}   [already there]
  '- update_submodule contrib/{openblas, faiss}                        [added here]
        '- git submodule update --init --recursive

be/CMakeLists.txt:1038  add_subdirectory(be/src/storage/index/ann)
  '- storage/index/ann/CMakeLists.txt:18  add_subdirectory(cmake-protect)
        '- cmake-protect/CMakeLists.txt:48,66  add_subdirectory(contrib/{openblas, faiss})
              '- target faiss --(PUBLIC)--> target ann_index --(PUBLIC)--> vector_search_test
                                                                              |
be/test/CMakeLists.txt:170  if (APPLE)                                        |
  |- before: -Wl,-force_load,<path to libvector_search_test.a>  -- propagates nothing
  '- after:  the same flag + ann_index                          -- propagates the faiss include dirs
                                                                              |
                                        function_map_inner_product_test.cpp <--'
                                        (and every other be/test TU, which now
                                         compiles with the same inputs as on Linux)

be/test/CMakeLists.txt  add_executable(doris_be_test ${UT_FILES})   <- the other six TUs, no link involvement

Release note

None

Check List (For Author)

  • Test

    • Manual test (add detailed scripts or steps below)

    Build. BUILD_TYPE_UT=Debug bash run-be-ut.sh on macOS 26.5 / arm64 / clang 20 / libc++: 1231 translation units, 0 failures, doris_be_test linked (340 MB after the dsymutil + llvm-strip post-build step). Before this PR the same command stops at configure, and after contrib/{faiss,openblas} are initialized by hand it stops at the first line of the table above and at six more after that.

    Tests. run-be-ut.sh --run with a filter over the affected suites: 43 tests from 5 suites ran, all passed — FormatRoundTest 8, FunctionMapInnerProductTest 11, BkdCorruptionTest 16, SniiSpimiLocalityBenchTest 3 (its InternSetReserveUpperBound is the case that exercises the key_equal fix, and it still asserts intern.size() == distinct), HierarchicalDataIteratorTest 5. The two bench files contain DISABLED_ cases only, so compilation is the whole check for them.

    Per-file compile check. Each of the seven TUs was also compiled on its own with the exact command line from the build's own compile_commands.json (clang++ -fsyntax-only ...), which is how the seventh was found without a full rebuild.

    Not run: Linux. The portability of each edit was argued from the Linux compile flags (libstdc++'s key_equal call order, LP64 PRIu64) rather than from a Linux build; the CI BE UT job covers that side.

  • Behavior changed:

    • No.
  • Does this need documentation?

    • No.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

morningman and others added 2 commits September 26, 2026 15:44
### What problem does this PR solve?

Issue Number: N/A

Problem Summary:

run-be-ut.sh initializes only three of the five submodules that build.sh
initializes: contrib/datasketches-cpp, contrib/apache-orc and
contrib/clucene. contrib/faiss and contrib/openblas are left out, but both
are required before a single file can be compiled: be/CMakeLists.txt adds
be/src/storage/index/ann unconditionally, and that directory's
cmake-protect/CMakeLists.txt calls add_subdirectory() on contrib/openblas
and contrib/faiss.

On a tree where those two submodules are not checked out yet -- a fresh
clone, or a new git worktree, which is the normal way to start work on this
repository -- run-be-ut.sh therefore dies in the configure step:

    CMake Error at src/storage/index/ann/cmake-protect/CMakeLists.txt:48 (add_subdirectory):
      The source directory .../contrib/openblas does not contain a CMakeLists.txt file.
    CMake Error at src/storage/index/ann/cmake-protect/CMakeLists.txt:66 (add_subdirectory):
      The source directory .../contrib/faiss does not contain a CMakeLists.txt file.
    -- Configuring incomplete, errors occurred!

Running build.sh once beforehand masks the problem, because build.sh does
initialize those two, which is why it is easy to miss.

Fix: add the two update_submodule calls, using the same archive URLs that
build.sh already passes.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Reproduced the configure failure on a tree with both submodules
      uninitialized, ran `git submodule update --init contrib/faiss
      contrib/openblas` by hand (the exact command the added lines run), and
      configure then completed and the build proceeded.
- Behavior changed: No
- Does this need documentation: No

Co-Authored-By: Claude Code <[email protected]>
…macOS

### What problem does this PR solve?

Issue Number: N/A

Problem Summary:

Seven be/test translation units fail to compile on macOS while the Linux BE UT
build stays green, so `run-be-ut.sh` on a Mac stops at the first of them and a
Mac developer cannot run the backend unit tests at all. run-be-ut.sh runs ninja
without -k, so the developer fixes one, waits for another full compile, and
hits the next one: seven round trips. None of the seven is related to a local
change -- a clean master reproduces all of them; the last one was introduced by
a commit merged after the previous sweep, which is exactly the case that only a
real build catches.

Root causes, one per file:

1. exprs/function/function_map_inner_product_test.cpp
   `fatal error: 'faiss/impl/platform_macros.h' file not found`.
   be/test/CMakeLists.txt links vector_search_test on APPLE with
   `-Wl,-force_load,$<TARGET_FILE:vector_search_test>`, which is a file path:
   it carries no usage requirements, so the faiss include directories that
   vector_search_test exports PUBLIC never reach this TU. The other platforms
   link `-Wl,--whole-archive vector_search_test`, which does propagate them.
   Fix: link ann_index explicitly in the APPLE branch as well. It is the target
   that exports the faiss include directories, and the resulting link line then
   matches what the other platforms already produce.

2. exprs/function/function_format_round_test.cpp
   `ambiguous conversion for functional-style cast from 'long' to 'Decimal64'
   (aka 'Decimal<long long>')`. Decimal64 is Decimal<int64_t>, so on Darwin the
   constructor parameter is `long long` while these decimal literals are `long`,
   leaving every integral and floating constructor an equally ranked conversion
   away. Fix: wrap the literals in int64_t(), the spelling already used by
   storage/key_coder_test.cpp and runtime/jsonb_document_test.cpp.

3. storage/index/snii/bench/bkd_native_vs_clucene_bench_test.cpp,
   six -Werror,-Wformat failures: uint64_t is `unsigned long long` on Darwin
   but `unsigned long` on Linux, so `%lu`/`%ld` is right on one and wrong on
   the other. Fix: print with PRIu64/PRId64.

4. storage/index/snii/bench/snii_vs_v3_benchmark_test.cpp
   - mincore(2) takes `char*` on Darwin and `unsigned char*` on Linux.
   - POSIX_FADV_DONTNEED / posix_fadvise do not exist on Darwin.
   Fix: one element type for the mincore buffer, and guard the posix_fadvise
   call.

5. storage/index/snii/bkd/bkd_corruption_test.cpp
   `deduced conflicting types ('size_type' (aka 'unsigned long') vs
   'uint64_t' (aka 'unsigned long long'))`: an initializer list mixing
   payload.size() (size_t) with uint64_t literals. Fix: hoist the size into a
   uint64_t first.

6. storage/index/snii/writer/spimi_locality_bench_test.cpp
   libc++ reports `no matching function for call to object of type
   'key_equal'`: the transparent equality functor only defined one argument
   order, and libc++ calls it as (key, lookup key) while libstdc++ calls it as
   (lookup key, key). The other order is added, and both are marked
   [[maybe_unused]] because either standard library leaves one of the two
   unused, which -Wunused-member-function (clang-only, and the Linux BE UT
   build also uses clang) turns into an error.

7. storage/segment/hierarchical_data_iterator_test.cpp
   `no matching function for call to 'min'`: kRows is size_t while
   current_ordinal is ordinal_t (uint64_t), distinct types on Darwin. Fix:
   narrow the remaining row count before the min() call.

On the other platforms the equivalent link line already puts vector_search_test
on the link list as a target, so every test TU is compiled with the faiss
include directories there; the APPLE branch now produces the same compile and
link inputs as the rest. The Linux build is otherwise unaffected: every other
change is either an APPLE-only path or a strictly more portable spelling of the
same value.

Note for macOS developers: because the APPLE link line now propagates include
directories, the compile command of every be/test translation unit changes
once, so the first build after this lands rebuilds be/test (~1230 TUs). Later
builds are incremental again.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - Each of the seven translation units was compiled on macOS with the exact
      command line taken from the build's own compile_commands.json
      (`clang++ -fsyntax-only ...`), not with a hand-written flag set.
    - Full macOS BE UT build: `BUILD_TYPE_UT=Debug bash run-be-ut.sh`, 1231
      translation units, 0 failures, doris_be_test linked.
    - Tests run on macOS: `run-be-ut.sh --run` with a filter over the affected
      suites -- 43 tests from 5 suites ran, all passed
      (FormatRoundTest 8, FunctionMapInnerProductTest 11, BkdCorruptionTest 16,
      SniiSpimiLocalityBenchTest 3, HierarchicalDataIteratorTest 5). The two
      bench files contain only DISABLED_ cases, so for them compilation is the
      whole check.
    - The remaining platform-only differences (libstdc++ vs libc++ key_equal
      call order, PRIu64 spelling) were verified against the compile flags of
      the Linux build rather than by building on Linux.
- Behavior changed: No
- Does this need documentation: No

Co-Authored-By: Claude Code <[email protected]>
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman

Copy link
Copy Markdown
Contributor Author

run beut

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Static review conclusion: request changes for one benchmark-correctness issue.

The clean-worktree bootstrap and the other macOS compile fixes are otherwise sound. The direct ann_index edge restores the required faiss/OpenMP usage requirements; the new submodule calls match build.sh; and the Decimal, format-macro, mincore, heterogeneous-lookup, corruption-length, and iterator-type edits preserve their values and contracts.

Critical checkpoints:

  • Goal and test proof: the PR largely makes the BE unit-test build portable, but the changed Apple cache path can publish warm local benchmark samples as cold. The author reports a full 1,231-TU macOS build plus 43 affected tests; this automated review was required to remain static, so those results were not independently executed here. The disabled SNII benchmark was compile-checked only and therefore did not test this semantic path.
  • Scope, reuse, and parallel paths: the changes are focused. run-be-ut.sh now matches the relevant build.sh submodule setup, and Apple/non-Apple CMake linkage and clean checkout/fallback paths were traced end to end.
  • Concurrency, lifecycle/static initialization, configuration, persistence/transactions, data writes, and FE-BE protocol compatibility: not involved by this test/build-only change.
  • Compatibility and error handling: the supported 64-bit ABI conversions and platform API signatures are correct. No storage, serialization, or rolling-upgrade format changes are introduced. The remaining conditional is the reported eviction gap.
  • Performance and observability: there is no production hot-path change. For benchmark validity, logging one file's residency without gating aggregation is insufficient when eviction is unavailable.
  • Tests and expected results: no generated result files change. The author-provided macOS build/test evidence covers compilation and ordinary affected suites, but not the disabled cold-cache benchmark semantics.
  • User focus: no additional focus points were supplied.

The review converged after two full normal/risk-focused rounds. All other candidates were dismissed with code evidence or duplicate-fenced.

}
::fsync(fd);
// posix_fadvise is Linux-only; elsewhere the fsync above is all this can do.
#if defined(POSIX_FADV_DONTNEED)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Do not report fsync-only samples as cold on macOS

On Apple this guard removes the only page-cache eviction operation. fsync(fd) writes dirty data but does not discard clean file pages, so _drop_caches() can leave the just-built/compacted rowset resident. The later _resident_pages() probe only prints one file's retained count and never fails or skips, and these samples are still aggregated under cold_query_p50/p99. That makes the local benchmark publish warm or partially warm results as cold on the platform this hunk enables. Please either evict the complete local query working set with a Darwin-supported mechanism, or fail/skip/unlabel the local cold phase before its samples are aggregated.

@morningman

Copy link
Copy Markdown
Contributor Author

skip buildall

@eldenmoon eldenmoon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@morningman
morningman merged commit 91d33b2 into apache:master Sep 28, 2026
56 checks passed
morningman added a commit that referenced this pull request Sep 28, 2026
### What problem does this PR solve?

Issue Number: N/A

Problem Summary:

When BE starts the CDC client, `CdcClientMgr` installs a process-wide
SIGCHLD handler. The handler previously called `waitpid(-1, ...,
WNOHANG)`, which could reap any child of BE. The embedded JVM also
starts subprocesses and waits for their exit codes. If the CDC handler
reaps one first, the JVM can observe `ECHILD` and report exit code 0
even when the command failed. One observed result was a failed
libc-detection probe appearing successful, causing RocksDB JNI to select
the musl library on a glibc host.

The handler now waits only for the CDC child. A generation-qualified
identity and an exclusive operation claim coordinate it with startup,
health inspection, and shutdown, so those paths cannot act on a PID
after it has been reaped and reused. A pending-reap handoff preserves
SIGCHLD notifications arriving during inspection. The latest review
identified another handoff window: an old handler could read generation
N before claiming, then claim N after N+1 was published and hide N+1's
only exit notification. The handler now rechecks the published
generation after releasing that stale claim. A deterministic test pauses
at both sides of this window and verifies that N+1 is reaped.

The affected tests now fail if their process setup fails, observe
SIGTERM through child-reported markers and phase handshakes instead of
`kill(pid, 0)`, and reap an unrelated direct child even when a fatal
handler checkpoint fails. The forced-shutdown test uses a direct child
that ignores SIGTERM; the handler/shutdown test waits for an actual
failed ownership claim; and the stale-generation helper starts with a
controlled signal environment.

The ownership rule changes what a failed startup does to its forked
child. The health-check failure path now terminates and reaps it, where
the PID used to be dropped and the child left running unmanaged. That
abandoned child could finish warming up and be adopted by a later
`start_cdc_client` call. On a host whose cold `cdc-client.jar` JVM
consistently answers `/actuator/health` later than the unchanged startup
window, pre-start the CDC client so the adoption path finds it, or give
the JVM more room. The cleanup also removes the unmanaged JVM left
holding `cdc_client_port`.

The macOS BE test compatibility changes, including the Decimal64 literal
fix, were merged separately in #68520. This branch was rebased onto that
merge and its duplicate test-only commit was dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants