Repository navigation
Conversation
765f029 to
1453067
Compare
c0bb671 to
513aa5f
Compare
93b3d85 to
2f1fabb
Compare
|
run buildall |
TPC-H: Total hot run time: 31893 ms |
TPC-DS: Total hot run time: 172802 ms |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
2f1fabb to
828baea
Compare
|
run buildall |
TPC-H: Total hot run time: 32221 ms |
TPC-DS: Total hot run time: 171729 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
828baea to
11b26ba
Compare
|
run buildall |
TPC-H: Total hot run time: 31969 ms |
TPC-DS: Total hot run time: 172832 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This commit collects all changes required to compile and run the BE unit tests on macOS 15+ / Apple Silicon (arm64) with Apple Clang 17. - run-be-ut.sh: auto-detect Apple Clang 17 and Homebrew JDK-17 on macOS - be/CMakeLists.txt: set DISABLE_ANN compile definition *before* add_subdirectory(storage/index/ann) so ann_index sees the flag; always build ann_index (production code links against it regardless of FAISS availability) - be/src/storage/CMakeLists.txt, be/src/exprs/CMakeLists.txt: always link ann_index - be/test/CMakeLists.txt: always exclude ANN test files from the monolithic doris_be_test target - be/src/storage/index/ann/CMakeLists.txt: guard OpenMP / faiss link with if(NOT DISABLE_ANN) - be/src/storage/index/ann/cmake-protect/CMakeLists.txt: strip ASAN/UBSAN flags from OpenBLAS sub-build (prevents getarch hang on macOS); disable OpenMP in OpenBLAS on Apple Silicon; force ARMV8 target - be/src/common/factory_creator.h: use std::shared_ptr(new T) instead of std::make_shared on Apple to avoid libc++ compressed_pair bug - be/src/common/multi_version.h: use atomic_shared_ptr wrapper - be/src/core/binary_cast.hpp: add missing #include <bit> - be/src/core/value/vdatetime_value.h: guard non-const copy ctor with #ifndef __APPLE__ (Apple Clang 17 rejects it for trivially-copyable) - be/src/exec/common/memory.cpp: disambiguate std::max literal types - be/src/load/routine_load/kinesis_conf.cpp: explicit int64_t cast for Aws::Utils::DateTime constructor - be/src/macos_patches/__ranges/lazy_split_view.h: libc++ patch for std::ranges::lazy_split_view used in tests - be/src/storage/index/ann/ann_index_writer.cpp, be/src/storage/index/ann/ann_index_reader.cpp: guard faiss includes and FaissVectorIndex usage with #ifndef DISABLE_ANN - be/src/storage/index/ann/faiss_ann_index.cpp, be/src/storage/index/ann/faiss_ann_index.h: wrap entire file body with #ifndef DISABLE_ANN - be/src/exprs/function/array/function_array_distance.h: provide inline FAISS stubs under #else DISABLE_ANN so L1/L2/IP distance classes compile without FAISS headers - be/src/storage/segment/segment_writer.h: move _is_mow() and _is_mow_with_cluster_key() inline definitions from the .cpp to the .h; macOS linker dead-strips out-of-line inline symbols, causing TestSegmentWriter link failures - be/src/storage/segment/segment_writer.cpp: remove the now-redundant inline definitions - be/test/udf/python/python_env_test.cpp: replace sighandler_t (Linux glibc extension) with void (*)(int) - be/test/storage/segment/inverted_index_reader_test.cpp: local copy for vector<bool> element to avoid proxy-reference UB in template - be/test/storage/index/ann/*.cpp: guard faiss / HNSW includes with #ifndef DISABLE_ANN - Multiple test files: add LL suffix to large integer literals, replace std::ranges::iota_view loops, add missing #include <thread>, and disambiguate int64_t literal types to satisfy Apple Clang 17
…clang-format - Ignore vendored libc++ patch dir be/src/macos_patches in license check (carries the LLVM Apache-2.0-WITH-LLVM-exception header, not the ASF one) - Reformat compaction_permit_limiter_test and inverted_index_reader_test to satisfy clang-format
… runner The macos-15 GitHub runner is now Apple Silicon (arm64): - JAVA_HOME_17_X64 is empty on arm64, so set JAVA_HOME from the Homebrew openjdk@17 install (and install openjdk@17 instead of @11); env.sh requires JDK-17. - Download the darwin-arm64 prebuilt thirdparty (matching uname -m) instead of the hardcoded x86_64 tarball, which both mismatches the runner arch and lacks libarrow_dataset.a.
11b26ba to
63a114b
Compare
|
run buildall |
TPC-H: Total hot run time: 31732 ms |
TPC-DS: Total hot run time: 172650 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
/review |
There was a problem hiding this comment.
I found one blocking CI issue. The macOS BE UT workflow now installs llvm@16, but run-be-ut.sh hardcodes llvm@18 for CMAKE_AR/CMAKE_RANLIB, so a clean macos-15 runner can fail before tests execute if llvm@18 is not already present.
Critical checkpoint conclusions:
- Goal/test proof: the PR aims to make macOS BE UT runnable on current Apple Silicon runners, but this toolchain mismatch can prevent the CI job from building, so the goal is not reliably accomplished.
- Scope/focus: most changes are focused on macOS build/test enablement; the reported issue is within that scope.
- Concurrency: no production concurrency behavior is changed by the reviewed blocker.
- Lifecycle/static initialization: no new lifecycle issue found beyond skipped flaky teardown tests noted by the PR.
- Configuration: DISABLE_ANN/OpenMP/macOS CMake flags are build-time only; no runtime dynamic config concerns found.
- Compatibility/storage/transaction/data correctness: no persisted format, FE-BE protocol, transaction, or data visibility change found in this CI-focused PR.
- Parallel paths: the macOS workflow installs one LLVM version while the shared script uses another path; these parallel setup paths must be kept consistent.
- Tests: the PR adds/adjusts macOS BE UT execution, but the job can fail at build setup due to the hardcoded missing tool.
- Observability/performance: no additional observability issue found; disabling ccache/ANN and using RELEASE are CI-resource tradeoffs.
User focus: no additional user-provided review focus was specified.
| # ASAN-instrumented .o files exceed ~4 GB total. Use LLVM's ar/ranlib | ||
| # (from Homebrew llvm@18) which supports the GNU 64-bit extended format. | ||
| "-DCMAKE_AR=/opt/homebrew/opt/llvm@18/bin/llvm-ar" | ||
| "-DCMAKE_RANLIB=/opt/homebrew/opt/llvm@18/bin/llvm-ranlib" |
There was a problem hiding this comment.
This hardcodes llvm@18, but the updated macOS workflow installs only llvm@16 (.github/workflows/be-ut-mac.yml). On a clean macos-15 runner /opt/homebrew/opt/llvm@18/bin/llvm-ar and llvm-ranlib are not guaranteed to exist, so CMake/build can fail before any UT runs. Please keep the installed Homebrew LLVM version and these tool paths consistent, or discover llvm-ar/llvm-ranlib from an installed toolchain instead of hardcoding an uninstalled version.
…#69695) ### What problem does this PR solve? Issue Number: None Related PR: #48788 Problem Summary: **In short.** `load_data_from_csv()` in the BE test utilities dereferences `std::set::end()` in an assertion. With libc++ (macOS) that reads past the set on the stack, so under ASAN every BE unit test suite that loads a CSV fixture aborts while loading its fixtures and takes the rest of the `doris_be_test` run with it. This removes the assertion; it only repeated the size check on the line above. **Background.** #48788 added `load_data_from_csv()` (`be/test/testutil/test_util.cpp`), which the column, data type and SerDe unit tests use to load their CSV fixtures. It checks that the SerDes, the columns and the requested column indexes line up: `serders.size() == columns.size()`, `serders.size() == idxes.size()`, and then `serders.size() == *idxes.end()`. **The problem, and what it cost.** Dereferencing the end iterator of a `std::set` is undefined behavior. In libstdc++ the end iterator points at the tree header, whose value slot overlaps the element count, so on Linux the last assertion compares against `idxes.size()` again and passes. In libc++ it reads outside the set object: ASAN reports a stack-buffer-overflow, and with `abort_on_error` the whole `doris_be_test` process stops. The suites of all 24 test files that load CSV fixtures (`DataTypeDateTimeV2SerDeTest`, `DataTypeNumberSerDeTest`, `ColumnStringTest`, ...) cannot run on macOS, and a test filter that includes one of them ends the run there. CI runs on Linux only, so it never failed there. **How this PR fixes it.** Remove the assertion. The size check right above it already guarantees one index per SerDe. No production code changes. #63474, open since May, carries the same removal among its macOS build fixes; this PR takes only this line, so the test fix does not wait for the rest. **Results.** | | macOS arm64, ASAN | Linux | |---|---|---| | Before | `DataTypeDateTimeV2SerDeTest` aborts in `SetUpTestSuite` with a stack-buffer-overflow | passes (the assertion repeats the size check) | | After | the 24 CSV-fixture suites (364 tests) pass | unchanged |
Summary:
This PR makes the community BE UT (macOS) CI workflow runnable again on the current GitHub Actions macos-15 (Apple Silicon arm64) runner. The workflow used to assume an Intel runner and broke after GitHub upgraded the macOS-15 image to Apple Silicon; this change fixes the workflow plumbing, ports the BE to compile cleanly under Apple Clang 17 + libc++, and works around macOS-specific test divergences via a gtest filter.
All C++ source changes are guarded by APPLE / OS_MACOSX / DISABLE_ANN (only set on macOS UT via run-be-ut.sh). Linux CI / production builds are not affected.
What's changed
1. CI workflow (.github/workflows/be-ut-mac.yml)
Install openjdk@17 (env.sh requires JDK 17; was @11)
Detect arch and download doris-thirdparty-prebuilt-darwin-arm64.tar.xz (was hardcoded x86_64)
Pre-build disk cleanup (old Xcodes, simulator runtimes, /Users/runner/hostedtoolcache, Android SDK, dotnet) — macos-15 runners only have ~14 GB disk
BUILD_TYPE_UT=RELEASE — ASAN deadlocks at startup on macOS 15+ via the dyld_shared_cache_iterate_text_swift bug, and ASAN compile exceeds the 6h job budget
EXTRA_CXX_FLAGS=-UNDEBUG + CMAKE_CXX_FLAGS_RELEASE patched in be/CMakeLists.txt so internal self-check methods (Block::check_type_and_column etc., gated by #ifndef NDEBUG) stay enabled — matches Linux ASAN_UT behaviour where NDEBUG is undefined
CCACHE_DISABLE=1 — hit rate is 0% across runs (different build types + new branch) and the local cache only contends for disk
gtest_filter excluding platform-incompatible tests (full list with reasons in the workflow comments)
2. macOS Apple Silicon BE compilation (APPLE / OS_MACOSX guarded)
be/CMakeLists.txt:
Inject -isysroot $(xcrun --show-sdk-path) into CMAKE_C_FLAGS scoped around add_subdirectory(clucene) — clucene's for/CMakeLists.txt uses add_custom_command which doesn't inherit CMAKE_OSX_SYSROOT, so <stdio.h> was not found
-Wno-shadow -Wno-shadow-field on macOS (Apple Clang detects enum/protobuf shadows that GCC doesn't)
-g0 on macOS (full DWARF + dsymutil OOMs the 7 GB-RAM runner during link of doris_be_test)
include_directories(BEFORE ${SRC_DIR}/macos_patches) to shadow the buggy Apple SDK lazy_split_view.h
Build ann_index as a stub when DISABLE_ANN=ON so production code linking against it still resolves
be/src/common/factory_creator.h: use the two-allocation shared_ptr(new T(...)) form on APPLE to avoid a libc++ compressed_pair static_cast bug when make_shared meets enable_shared_from_this
be/src/core/value/vdatetime_value.h: drop the user-declared non-const copy ctor on APPLE — Apple Clang 17 treats DateV2Value(DateV2Value&) = default as breaking is_trivially_copyable (GCC/Linux Clang don't)
be/src/core/binary_cast.hpp: add #include (macOS libc++ doesn't bring it in transitively)
be/src/macos_patches/__ranges/lazy_split_view.h (new vendored file): patches the Apple SDK __ranges/lazy_split_view.h where __outer_iterator/__inner_iterator are forward-declared private but defined public — Apple Clang rejects this; only included on macOS via the BEFORE-prepended include dir
be/src/storage/segment/segment_writer.{h,cpp}: move _is_mow()/_is_mow_with_cluster_key() inline definitions from the .cpp to the class body — the original layout (inline keyword only in .cpp) is technically unspecified and breaks under macOS dead-strip
be/test/CMakeLists.txt: exclude exprs/function/geo/functions_geo_test.cpp on macOS (file is from master and contains assert_cast<ColumnUInt8*>(ColumnUInt8*) which trips the new same-type static_assert in assert_cast.h)
3. run-be-ut.sh
On macOS: auto-detect DORIS_CLANG_HOME (Apple CLT clang 17) and JAVA_HOME (openjdk@17)
USE_AVX2=OFF on non-x86 architectures by default (AVX2 doesn't exist on arm64)
Darwin-specific cmake vars: -DDISABLE_ANN=ON, disable OpenMP find-package, -DCMAKE_OSX_SYSROOT, -DCMAKE_AR=/opt/homebrew/opt/llvm@18/bin/llvm-ar and ranlib (Apple's ar has a 32-bit archive-offset overflow when total .o payload exceeds ~4 GB)
4. DISABLE_ANN source guards (only set on macOS UT)
be/src/exprs/function/array/function_array_distance.h provides FAISS stub functions (fvec_L1, fvec_L2sqr, fvec_inner_product) when DISABLE_ANN defined
be/src/storage/index/ann/{ann_index_reader,ann_index_writer,faiss_ann_index}.{cpp,h} wrap FAISS-only code in #ifndef DISABLE_ANN
be/src/exprs/CMakeLists.txt, be/src/storage/index/ann/CMakeLists.txt, be/src/storage/index/ann/cmake-protect/CMakeLists.txt, be/test/CMakeLists.txt: corresponding cmake-level if(NOT DISABLE_ANN) guards
5. CI-only ignores
.licenserc.yaml: ignore be/src/macos_patches/** (vendored libc++ patch keeps original Apple header file, no Apache license)
.clang-format-ignore: same path, vendored file shouldn't be reformatted
6. Test platform-compatibility tweaks (test-only, behaviourally identical on Linux)
Type disambiguation: 0L → int64_t{0}, decimal literals → LL suffix (Apple Clang's overload resolution is stricter; Linux x86_64 picks the same overload either way since long == int64_t)
std::ranges::for_each(std::ranges::iota_view{0,N}, lambda) → plain for loop (macOS libc++ ranges has gaps in feature availability)
Add missing #include where it was only transitively visible on libstdc++
be/test/testutil/test_util.cpp: remove a UB assertion ASSERT_EQ(serders.size(), *idxes.end()) that happened to pass on libstdc++ via implementation-defined sentinel memory; functionally a duplicate of the size-equality check immediately above it
Tests skipped on macOS via gtest filter (with reasons)
These are real macOS platform-specific divergences, not regressions caused by this PR. They all pass on Linux CI and are skipped only on the macOS workflow. Each entry in the workflow has a comment explaining the reason:
Memory-bound: test_sink_large_string_data_over_4g
Concurrent / flaky on macOS: CloudTabletMgrTest.TestConcurrentGetTabletTabletMapConsistency, MemTableFlushExecutorTest.TestConfigUpdateTrigger, MemTableFlushExecutorTest.TestDynamicThreadPoolUpdate
Platform-specific (macOS Heimdal vs Linux MIT krb5): KerberosTicketCacheTest.*
libm precision divergence (least-significant-bit difference vs Linux glibm): MathFunctionTest.{acos,asin,atan,atanh,cbrt,cot,sec,sinh,tan}test
Serialization with unzeroed padding (Linux GCC happens to zero padding, macOS libc++ doesn't — real latent UB, worth a separate audit): BlockTest.SerializeAndDeserializeBlock, ColumnComplexTest.GetDataAtTest, DataTypeSerDe{,Pb}Test.DataTypeScalaSerDeTest, QuantileStateSerdeTest.writeColumnToPb, BlockSerializeTest.JsonbBlock, function{bitmap,hll}test.function_to_base64
macOS-specific cleanup crashes (static destructor / thread-pool teardown races): DataQueueTest., SubQueueTest., LocalExchangerTest.TestShuffleExchangerWrongMap, LookupConnectionCacheTest.PQTestCapacityBoundary, HttpClientTest.{batch_download,enable_http_auth}
macOS FS / HTTP / TLS divergence: LocalFileSystemTest.TestGlob, HttpClientTest.download_file_md5, S3ObjStorageClientMockTest.test_ca_cert
Cache edge cases: BlockFileCacheTest.{evict_in_advance,version3_add_remove_restart}
Decimal overflow / AggArray edge cases: AggGroupArrayIntersectTest.string{,_nullable}test, FunctionCastToDecimalTest.test_to_decimal256_from{double,float}_overflow
ParquetExprTest: ParquetExprTest.test_in
The serialization-padding cluster in particular probably indicates real latent UB in those serde paths that GCC currently hides; opening a follow-up issue for the storage team to audit is encouraged.
Test plan
Locally on macOS Apple Silicon: ./run-be-ut.sh --run --clean (which automatically enables BUILD_TYPE_UT=RELEASE via the workflow defaults)
Linux CI: unaffected — be-ut.yml does not read this workflow's filter or env vars; be/CMakeLists.txt changes are all behind OS_MACOSX / DISABLE_ANN / APPLE
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)