Repository navigation
[fix](be) Route glibc allocation entry points through jemalloc - #69819
Merged
Merged
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Before glibc 2.25, the dynamic loader allocates TLS for
late-loaded native libraries with __libc_memalign and releases it through
public free. BE redirects public allocation functions to jemalloc but
leaves this internal entry point on libc, so thread cleanup can pass a
libc allocation to jemalloc and crash the BE. Alias __libc_memalign to the
existing doris_memalign implementation.
Add standalone native allocator tests through run-be-ut.sh --jemalloc-hook.
The regular BE UT disables jemalloc and uses sanitizer interceptors, so it
cannot validate this allocation/free pair. The tests cover the legacy
pair in a subprocess, alignment and error semantics, cross-thread frees,
and dynamic TLS cleanup from a late-loaded shared library.
### Release note
Fix a BE crash when threads clean up dynamic TLS from native libraries on
older glibc systems.
### Check List (For Author)
- Test: Unit Test - ./run-be-ut.sh --jemalloc-hook --clean --run -j 2
- Four native tests passed on glibc 2.32; the unpatched hook triggers
SIGSEGV in the legacy allocation/free subprocess.
- clang-format v16, clang-tidy, build hygiene, and bash syntax checks passed.
- CentOS 7 / Jindo / DLF end-to-end testing was not run.
- Behavior changed: Yes - legacy glibc TLS allocations now use jemalloc
consistently with their deallocation.
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
hubgeter
marked this pull request as draft
October 29, 2026 07:40
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
TPC-H: Total hot run time: 28336 ms |
yiguolei
reviewed
Oct 29, 2026
| // and release it through free. Route both to jemalloc so thread exit does not | ||
| // pass a glibc allocation to doris_free. | ||
| void* __libc_memalign(size_t align, size_t size) __THROW ALIAS(doris_memalign); | ||
| #endif |
Contributor
There was a problem hiding this comment.
does jemalloc hook still useful?
Contributor
Author
There was a problem hiding this comment.
Current master still includes this hook in Linux builds with USE_JEMALLOC=ON, and jemalloc still uses the je symbol prefix. I ran a minimal link test using the current master sources: with the hook, public malloc/free calls resolve to the jemalloc wrappers; without it, they resolve to libc. So the hook still provides allocator interposition. This PR fixes the mismatch between TLS allocation and deallocation under the existing configuration.
Contributor
TPC-DS: Total hot run time: 151332 ms |
Contributor
ClickBench: Total hot run time: 24.26 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
/review |
hubgeter
marked this pull request as ready for review
October 30, 2026 06:50
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review of PR #69819 at head b2e65a8 is complete. The change's production goal is coherent: on pre-2.25 glibc, the hook supplies the documented two-size_t __libc_memalign entry point and production CMake includes the hook only for Linux jemalloc builds. The standalone native tests cover aligned allocation, error behavior, thread-exit frees, and late-loaded TLS, but their symbol lookup is not tied to the executable definition; see the inline P2 finding.
Critical checkpoints: the production alias is a small, focused change with no configuration, thrift/protobuf, storage, transaction, or FE/BE protocol impact. The test exercises concurrent threads with per-index writes and joins before reads; the dlopen handle remains alive through worker-thread teardown and is released afterward. No new background thread, lock, tracked allocation, status path, static cross-TU initializer, or observability surface was added. Linux/glibc gating, jemalloc-only selection, macOS/non-jemalloc exclusion, runner option parsing, CMake library discovery, and the XML output path were rechecked. The XML concern was dismissed because the bundled GoogleTest writer creates parent directories. No additional user-provided focus was supplied.
This invocation performed static review only; it did not run builds or tests. One new P2 finding is included below. Existing inline comments were reviewed; neither is a P0/P1 finding that still applies, so existing_blocking_comment_ids is empty. The review converged in round 2 after the full changed-file sweep and duplicate-aware rechecks.
Review converged after 2 round(s).
| using MemalignFunction = void* (*)(size_t, size_t); | ||
|
|
||
| void SetUp() override { | ||
| void* symbol = dlsym(RTLD_DEFAULT, "__libc_memalign"); |
Contributor
There was a problem hiding this comment.
[P2] Ensure this test actually calls the hook defined by this PR. jemalloc_hook_test intentionally leaves ENABLE_EXPORTS off, but SetUp() obtains __libc_memalign only through dlsym(RTLD_DEFAULT). That lookup searches the dynamic symbol scope and can select libc's implementation (or vary with the linker), while the ordinary alias in the executable is not guaranteed to be visible. In that case the allocation/free assertions and the TLS test can pass without exercising the new alias. Call the linked hook directly or assert with dladdr/the executable's dynamic symbol table that the resolved address is the hook, and keep the DSO path tied to that verified interposition.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69819 Problem Summary: The native allocator tests used RTLD_DEFAULT without proving that the resolved symbols belonged to the linked production hook. A matching libc allocation/free pair could otherwise pass without testing jemalloc interposition. Anchor dladdr checks to doris_memalign and verify that __libc_memalign, malloc, and free come from the executable. Also check the late-loaded DSO's actual allocator bindings before exercising TLS. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --jemalloc-hook --run -j 2 (4/4 passed) - Removing the alias fails the executable-owner assertion. - Resolving the DSO allocator from libc fails the DSO binding assertion. - clang-format v16, clang-tidy, build hygiene, and git diff --check passed. - Behavior changed: No production behavior change. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 28469 ms |
Contributor
TPC-DS: Total hot run time: 151276 ms |
Contributor
ClickBench: Total hot run time: 24.19 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Reviewed all five changed files in PR #69819 at 71e74a911844e449ab103691a1197b285f765810. One new P2 is included inline: the standalone allocator test omits libunwind's zlib dependency on Linux ARM64 with modern ELF headers. No P0/P1 finding was identified.
The production alias follows the documented pre-2.25 allocator replacement requirement and pairs legacy TLS allocations with the existing jemalloc free hook. This is supported by the glibc allocator replacement contract and the glibc 2.17 TLS allocation/cleanup implementation.
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and proof | The alias supplies the missing legacy allocation entry point. Four native tests cover the intended allocation and TLS behaviors; their source was inspected, not executed. |
| Scope and reuse | Focused production alias reuses doris_memalign; native tests and runner dispatch are isolated. |
| Concurrency and locks | No new production shared state or locks. Workers write separate elements; joins synchronize reads and frees. No new lock-order issue found. |
| Lifecycle and initialization | Workers finish before DSO closure. The alias has no initializer; no new ownership cycle or cross-TU initialization dependency found. |
| Configuration | No Doris configuration added. The new runner option validates incompatible test modes. |
| ABI and compatibility | Additive glibc-only symbol, with no new production undefined libc API. Archive selection, selective exports, and legacy symbol-version lookup were traced. Final release ELF compatibility was not audited. |
| Thrift/protobuf | No changes; cross-branch field-ID review is not applicable. |
| Parallel paths | macOS/non-jemalloc hook exclusion remains coherent; regular ASAN UT excludes the standalone directory. Newer glibc retains its existing malloc/free path. |
| Conditions and comments | The glibc guard and native test dispatch explain the concrete allocator mismatch and sanitizer constraints. |
| Test coverage | Inspected hook identity, alignment/zero/non-multiple sizes, EINVAL/ENOMEM, cross-thread free and late-loaded TLS. ARM64 native linkage needs the inline fix. No legacy-glibc end-to-end run performed. |
| Expected test results | Assertions were inspected; no golden result files changed and no execution results are claimed. |
| Observability | Existing allocator behavior is reused; test diagnostics identify symbol-resolution failures. No additional production logging or metrics requirement found. |
| Transactions and persistence | No affected code; not applicable. |
| Data writes and atomicity | No database write-path changes; allocator and worker memory lifetimes were reviewed. |
| FE/BE variable passing | No variables or protocol fields added; not applicable. |
| Performance | Alias adds no per-call lookup, allocation, copying, or locking. |
| Other checks | No header/PCH/template/unity-gate changes. Static-library dependency review produced the single inline finding. |
Existing P2 comment 4422480037 is addressed at this head: fixture setup anchors lookup to the linked hook image and the DSO checks its actual allocator relocations. It is not reposted. The hook-usefulness discussion was checked against current Linux jemalloc inclusion. No existing P0/P1 finding applies, so existing_blocking_comment_ids is empty. No additional user focus was supplied.
All three reviewers returned NO_NEW_VALUABLE_FINDINGS in round 2. Every risk and candidate has an evidence-backed disposition, and the final five-file coverage and inline-anchor sweep is complete. No unresolved static-review question or uncovered changed file remains. Validation limits: no builds, tests, benchmarks, completed ELF audits, or CentOS 7/Jindo end-to-end execution were performed; repository source was not modified.
Review converged after 2 round(s).
| # executable's default symbol visibility. | ||
| target_link_libraries(jemalloc_hook_test PRIVATE | ||
| "${gtest_main_LIBRARY}" "${gtest_LIBRARY}" "${jemalloc_doris_LIBRARY}" | ||
| "${unwind_LIBRARY}" "${lzma_LIBRARY}" Threads::Threads ${CMAKE_DL_LIBS}) |
Contributor
There was a problem hiding this comment.
[P2] Link zlib for the standalone ARM64 test. On Linux aarch64 with modern ELF headers (SHF_COMPRESSED), the repository's default libunwind 1.8.3 build enables debug-frame and zlib support, so its dwarf lookup object references uncompress. The prof-enabled jemalloc dependency pulls that object in through unw_backtrace and local address-space initialization even when profiling is disabled at runtime. This target links libunwind and liblzma but never libz, so ./run-be-ut.sh --jemalloc-hook --run fails at link time with an undefined reference to uncompress, before the tests can run. Find the installed z library and link it after unwind, or consume dependency metadata that carries it.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69819 Problem Summary: Static libunwind builds with compressed debug-frame support can reference zlib's uncompress, including Linux ARM64 builds. The standalone allocator tests linked unwind and lzma but omitted zlib, causing a link failure before tests could run. Find zlib in the same thirdparty installation and link it after libunwind and liblzma. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --jemalloc-hook --run -j 2 (4/4 on x86_64) - Manual link probe: an injected uncompress reference fails without the dependency and links successfully with installed libz after the fix. - Verified the final link order and git diff --check. - Native ARM64 execution was not performed. - Behavior changed: No production behavior change. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28548 ms |
Contributor
TPC-DS: Total hot run time: 151432 ms |
Contributor
ClickBench: Total hot run time: 24.47 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69819
commit: cfee583affa5b48294e55e2c28e645ee4a7efe28
base: 8d88f1cc6f738a331ecfe024a2f1560c770068bb
reviewed_at: 2026-11-02T17:27+08:00
reviewer: hubgeter
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Problem Summary: Keep glibc's exported allocation entry points on the same jemalloc wrappers as the public interfaces, so callers can mix allocation, reallocation and release entry points safely. Preserve realloc failure errno and valloc/pvalloc page alignment and rounding semantics. Extend native tests to verify symbol ownership, cross-entry operations, boundary cases and late-loaded DSO bindings. ### Release note Keep glibc allocation entry points paired with jemalloc and preserve allocation API semantics. ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --jemalloc-hook --run -j 2 passed 12/12 on Linux x86_64/glibc 2.32; the new binding check rejects the previous hook. Full BE, older glibc, ARM64 and customer JNI E2E were not run. - Behavior changed: Yes. Route the remaining glibc allocation entry points to jemalloc and preserve allocator boundary semantics. - Does this need documentation: No
hubgeter
changed the title
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review of PR #69819 at head eed21b8 found no new inline finding that is verified for this head.
Critical checkpoints: the production change is focused on Linux jemalloc/glibc allocator interposition. The realloc errno repair, valloc/pvalloc alignment and overflow paths, zero-size behavior, allocator ownership, and the additive _libc* aliases were traced through the hook and standalone tests. No new production thread, lock, tracked allocation, status path, static initializer, configuration, transaction/persistence path, thrift/protobuf field, FE/BE variable, or observability surface was added. The existing macOS and non-jemalloc exclusions remain coherent, and the runner's standalone dependency discovery and option parsing were reviewed. The native tests cover cross-entry allocation, realloc, alignment, overflow, thread-exit frees, symbol ownership, and late-loaded TLS; their source was inspected only.
Existing review context was revalidated at this head. The prior hook-identity concern is addressed by the dladdr checks and the DSO relocation comparison. The prior ARM64 zlib linkage concern is addressed by linking z. The XML-output-directory concern raised in the prior review body was rechecked there and is not reposted. No existing P0/P1 inline finding still applies, so existing_blocking_comment_ids is empty. No additional user-provided focus was supplied.
This was a static review only; no builds, tests, benchmarks, ELF audits, or end-to-end runs were executed. The review reached the third-round delivery limit. The main agent completed the final changed-file and anchor sweep and found no additional issue, but the required symbol/DSO-focused subagent could not return after repeated model-capacity failures. The delivery is therefore explicit and verified as converged=false; this remaining subagent-coverage gap is the incomplete reason.
Review did not converge after 3 rounds. This automated review cannot pass the review check.
The symbol/DSO-focused subagent did not return after repeated model-capacity failures. The main agent directly rechecked that scope and found no additional issue, but the contract requires every assigned latest-round subagent to return NO_NEW_VALUABLE_FINDINGS before convergence can be true.
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69819
commit: eed21b8c1e0e84327d4a4c78c017a797ce90fcc5
base: 8d88f1cc6f738a331ecfe024a2f1560c770068bb
reviewed_at: 2026-11-03T13:13+08:00
reviewer: hubgeter
model: gpt-6.1-sol
effort: max
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
TPC-H: Total hot run time: 29455 ms |
Contributor
TPC-DS: Total hot run time: 151637 ms |
Contributor
ClickBench: Total hot run time: 24.41 s |
yiguolei
approved these changes
Nov 3, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
github-actions Bot
pushed a commit
that referenced
this pull request
Nov 4, 2026
### What problem does this PR solve?
Problem Summary:
On glibc < 2.25, dynamic TLS can be allocated by libc and freed by
jemalloc, crashing during thread cleanup. Route all seven glibc
allocation entry points through the existing jemalloc wrappers, preserve
allocator API semantics, and add native interposition tests.
### Release note
Fix a BE crash during dynamic TLS cleanup on older glibc systems and
keep glibc allocation entry points paired with jemalloc.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
`./run-be-ut.sh --jemalloc-hook --run -j 2`: 12/12 passed on Linux
x86_64/glibc 2.32. Full BE, older glibc, ARM64 and customer JNI E2E were
not run.
- Behavior changed:
- [ ] No.
- [x] Yes. Glibc allocation entry points now consistently use jemalloc,
with allocator boundary semantics preserved.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
github-actions Bot
pushed a commit
that referenced
this pull request
Nov 4, 2026
### What problem does this PR solve?
Problem Summary:
On glibc < 2.25, dynamic TLS can be allocated by libc and freed by
jemalloc, crashing during thread cleanup. Route all seven glibc
allocation entry points through the existing jemalloc wrappers, preserve
allocator API semantics, and add native interposition tests.
### Release note
Fix a BE crash during dynamic TLS cleanup on older glibc systems and
keep glibc allocation entry points paired with jemalloc.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
`./run-be-ut.sh --jemalloc-hook --run -j 2`: 12/12 passed on Linux
x86_64/glibc 2.32. Full BE, older glibc, ARM64 and customer JNI E2E were
not run.
- Behavior changed:
- [ ] No.
- [x] Yes. Glibc allocation entry points now consistently use jemalloc,
with allocator boundary semantics preserved.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
3 of 12 tasks
yiguolei
pushed a commit
that referenced
this pull request
Nov 6, 2026
### What problem does this PR solve? Issue Number: None Related PR: #65730 #66608 #66805 #66841 #66999 #67005 #67011 #67110 #67268 #67289 #67325 #67650 #67921 #68028 #68335 #68375 #68615 #68690 #68813 #68815 #68707 #68649 Problem Summary: **In short.** This ports the Lance BE read path of branch-4.1 / branch-4.2 to master, in its branch-4.2 form as of `cabcf45c1af`, plus the BE side of three 4.x changes merged after that snapshot: the search filter `prefilter` flag and profile layout (#68813), distance bounds for `vector_search` (#68815), and the split-level index rule of #68707 that #68815 relies on. It gathers the BE parts of the PRs listed above. Master links `liblance_c` but has no Lance reader, so every `FORMAT_LANCE` scan fails today. With this PR the BE reads Lance datasets through FileScannerV2: plain scans, vector search and full-text search, predicate and runtime-filter pushdown, the schema fetch of a local Lance TVF, and the second phase of TopN lazy materialization. The PR is BE only. The FE connector plugin and the regression suites come in separate PRs, so until then nothing on master plans a Lance scan, and the BE unit tests carry the proof. Original authors: @zhangstar333 @Gabriel39 @wangzhaobo957-cloud @Jay-ju @FANNG1 **Background.** Lance shipped in 4.1.4: the Lance catalog, `local()` / `s3()` with `format=lance`, and the `vector_search` / `full_text_search` TVFs. On master the FE side is being rebuilt as a connector plugin (`fe-connector-lance`), and the BE needs the reader from 4.x. The thrift ids are the branch-4.1 / 4.2 ones, including `TExternalSearchRequest.5` (`prefilter`) and `TVectorSearchParams.6/7` (distance bounds) added after the snapshot, so a BE built from master also reads the Lance scan ranges and search requests that a 4.x FE sends during a rolling upgrade. The one exception is `TVectorSearchOptions` 6, branch-4.2's `approx_mode` (#69852, merged into branch-4.2 after the snapshot and not ported): it is reserved in a comment, and a master BE ignores it, so during a rolling upgrade a 4.2 FE that asks for `fast` or `accurate` gets Lance's default (`normal`). **The problem, and what it cost.** | | master today | with this PR | |---|---|---| | A `FORMAT_LANCE` scan | FileScannerV2 has no Lance reader, so the scan fails with "FileScannerV2 does not support file format" | read by `LanceTableReader` in FileScannerV2 | | `fetch_table_schema` for a local Lance TVF | not handled | returns the Arrow schema mapped to Doris types; Null leaves keep `NULL_TYPE` | | The row location of a Lance row in TopN lazy materialization | `GlobalRowLoacationV2` only has a 32-bit row id, but a Lance row id is a 64-bit id within the snapshot | a version 1 layout carries the 64-bit row id; phase two reads the rows with the dataset's `take_rows` | **How this PR fixes it.** - Reader: `be/src/format_v2/lance/*` and `be/src/format_v2/table/lance_reader.*`, as on branch-4.2 except one local renamed for master's `-Wshadow`. The unit tests construct `RuntimeState(TQueryOptions(), globals)`, because master removed the globals-only constructor. - FileScannerV2: the reader dispatch and `_to_file_format` handle `FORMAT_LANCE` / `"lance"`; `FileFormat::LANCE` is added to its two exhaustive switches. Since #69723 every file scan uses FileScannerV2, so nothing else routes Lance scans (on branch-4.2 a Lance query fails with NotSupported when `enable_file_scanner_v2` is off). - Row locations: `ROW_VERSION {FILE_LOCAL_ROW_ID = 0, LANCE_DATASET_ROW_ID = 1}` and a union layout for `GlobalRowLoacationV2`. Version 0 keeps its byte layout, which static asserts pin. `PRequestBlockDesc.row_id` becomes `uint64` (a proto2 varint, so the wire format is compatible). The materialization operator decodes row locations by version, rejects an unknown version or a wrong size, and stamps `row_location_version`. `RowIdStorageReader` validates the counts and the version, range-checks internal row ids against `uint32`, and groups Lance rows by plan node, snapshot version and dataset URI, so each dataset is read with one `take_rows`. Master's segment batching, scan-slot dedup and fetch concurrency stay as they are. - Thrift (`PlanNodes.thrift`): the external search request body (`TVectorElementType`, `TVectorMetric`, `TSearchVector`, `TVectorSearchParams` 1-7, `TFtsCoverageMode`, `TFtsQueryType`, `TFtsMatchOperator`, `TFullTextSearchParams`, `TSearchFilterFormat`, `TSearchFilter`, `TVectorSearchOptions`, `TExternalSearchQuery`), `TExternalSearchRequest` 2-5 and `TLanceFileDesc` 4-6, all with the branch-4.1 / 4.2 ids. The enums spell out their 4.x values. No id is reused. On master, #70075 reserved `TExternalSearchRequest` 2-5 in comments and added 6 (`paimon_options`); this PR replaces those comments with the fields and leaves 6 as it is. `TVectorSearchOptions` 6 is reserved in a comment for branch-4.2's `approx_mode` (#69852). - After the snapshot (one commit each): - #68813: `prefilter` decides whether the TVF search filter runs before (true) or after (unset / false) candidate selection, for vector and full-text search. The profile groups the Lance counters under `LanceTiming`, `LanceIO`, `LanceIndex`, `LanceVectorIndex`, `LanceScalarIndex`, `LanceExecutionPlan`, `LanceScan`, `LanceSearchPlan` and `LanceDataCache`, uses the 4.x counter names, and registers every scan metric before the statistics callback is installed, so the callback, which Lance invokes from its execution streams, never adds counters. #68813's FE tweak that hides `TUnit.NONE` labels in `AggCounter` belongs to the FE port. - #68707 (part): a vector split searches exactly the index segments FE planned for it, and a split without segments is searched flat whatever `use_index` says; a split with segments under `use_index = false` is rejected. 4.x FEs plan segments only when `use_index` allows them. - #68815: `distance_lower_bound` (inclusive) and `distance_upper_bound` (exclusive) are validated (finite FLOAT values, lower < upper at FLOAT precision, single-vector only) and installed with `lance_scanner_set_distance_range()`, which master's pinned lance-c provides. - Review fixes: - A nested array goes to the Doris SerDes without a copy only when it and every descendant start at offset 0 and reference exactly their child ranges. Lance's legacy (v1) decoder can return a struct or list that is tight at its own level over a Boolean child with a bit offset (a batch that starts mid-byte in the bitmap), and the SerDes reject that under Arrow input validation. - A zero-field `STRUCT<>`, which Lance decodes, is reported as an unsupported column at any nesting level, like the other unsupported Lance types. It used to be typed as a STRUCT that `ColumnStruct` cannot hold (its row count comes from its first child). - A Lance Decimal256 field with precision <= 38 is narrowed to Decimal128 by a safe Arrow cast before decoding. Doris types it DECIMAL32/64/128I by precision, and those SerDes read only Decimal128 arrays; the BE used to crash on the first value, also for nested leaves. Only values a reader can see are cast: before a nested leaf is narrowed, every child slot that no visible row references (under a NULL list, fixed-size-list, map or struct row, or outside a sliced parent) is marked NULL, so a hidden value does not fail the cast and a visible value outside the declared precision still does; map keys, which cannot be NULL, stay checked. - FLOAT / DOUBLE `>=` runtime filters stay residual-only. Lance compares floats by totalOrder, so a pushed `x >= c` dropped negative-NaN rows that Doris keeps. - Runtime filters that mention INT64_MIN stay residual-only. The pinned Lance SQL parser reads `-9223372036854775808` as the negation of a magnitude that it parses as i64 first; the overflow turns the literal into a Float64, which the planner cannot coerce to Int64 or Decimal, so a pushed IN, `>=` or `<=` filter on an INT64 or scale-zero decimal column failed planning and the query failed. Every other i64 is still pushed. - The shared Arrow DATE32 decoding (`DataTypeDateV2SerDe::read_column_from_arrow`) rejects days outside 0000-01-01..9999-12-31 instead of returning a non-null `0000-00-00`, skips null slots and adds the epoch in 64 bits: the DATE32 range part of branch-4.1 / 4.2 #68649. It also changes ADBC, Arrow stream load and paimon-rust reads. - Child values hidden by a NULL list, fixed-size-list, map or struct row are not decoded: the array, map and struct SerDes give the element SerDe a validity bitmap that also clears those slots (`mask_arrow_children_hidden_by_null_parents()`). Arrow leaves them unspecified, and Lance's v2.1 miniblock encoding keeps them, so a hidden 10000-01-01 under a NULL fixed-size list used to fail the DATE32 range check above. Visible values are unchanged; this applies to ADBC, Arrow stream load and paimon-rust as well. The masks come from ExecEnv's tracked Arrow pool. A nested Lance array that is compacted before decoding is masked the same way, at every level, before its `ValidateFull()`, so a hidden invalid UTF-8 string, out-of-precision decimal, Date64 or time no longer fails the batch while a visible one still does. Since #69697 the same masks keep hidden DATETIMEV2 / TIMESTAMPTZ values out of its range checks. A mask costs at most one pass per batch: a NULL row that owns no child slots, or only NULL ones, allocates nothing, and Arrow stream load masks each retained IPC batch once (`mask_arrow_descendants_not_visible()`) before it hands row ranges to the SerDes, so its per-block reads allocate nothing. The one per-block mask left is for map keys under a NULL map row that still owns entries, since a MapArray cannot hold NULL keys; Arrow's map builder writes NULL map rows without entries. - Core: `to_protobuf` keeps `NULL_TYPE` for a NULL literal type, so a Null leaf in ARRAY / MAP / STRUCT no longer comes back as BOOLEAN. The nullable serde reads an Arrow Null array as NULLs without touching buffers that the array does not have. - Configs, caches and metrics, with the 4.x names and defaults: | Config | Default | |---|---| | `lance_index_cache_size_bytes` | 10 GiB | | `lance_metadata_cache_size_bytes` | 1 GiB | | `enable_lance_data_cache` | true | | `lance_data_cache_path` | `${DORIS_HOME}/lance_data_cache` | | `lance_data_cache_disk_capacity_bytes` | 100 GiB | | `lance_data_cache_read_block_size_bytes` | 1 MiB | | `lance_io_buffer_size_bytes` (mutable) | 2 GiB | | `lance_batch_readahead` (mutable) | 5 | | `lance_fragment_readahead` (mutable) | 5 | The BE-wide Lance session (index cache, metadata cache and the optional Foyer disk cache) is created by the first Lance scan, so a BE that never reads Lance creates no cache directory. The first six configs take effect after a restart. A negative cache size (and, while the data cache is enabled, a negative disk capacity or read block size) fails the first Lance scan with a message that names the setting, as does any data-cache value lance-c rejects; the rest of the BE keeps serving. Ten `lance_session_*` metrics report the two in-memory caches. With the defaults, the disk tier keeps 51,200 sparse 2 MiB block files open from the first Lance scan on (100 GiB in blocks of twice the read block size), the same as on branch-4.1 / 4.2. Size `lance_data_cache_disk_capacity_bytes` and `lance_data_cache_read_block_size_bytes` against the BE's open-file limit, or set `enable_lance_data_cache = false`. The index and metadata caches live in lance-c memory: they are not charged to a MemTracker or registered with CacheManager, so memory GC, the cache capacity adjustment under memory pressure and `/api/clear_cache/all` do not shrink them, and `enable_lance_data_cache = false` does not turn them off. lance-c has no call to clear, evict or resize them, so size them with the two configs. The disk tier keys blocks by Lance's store prefix (`scheme$bucket` for S3, GCS and OSS, without the endpoint) and the object path (`thirdparty/patches/lance-c-foyer.patch`). Several endpoints that serve the same bucket name and data-file path with different bytes (data files registered under chosen names or replaced in place; Lance's writers use random file names) share entries, also across a BE restart; such a deployment should set `enable_lance_data_cache = false`. Both are as on branch-4.1 / 4.2 and tracked as follow-ups. A Lance scan's native working set, that is the I/O buffered up to `lance_io_buffer_size_bytes` per scanner, the decode readahead and the operators of a search plan such as the top-k of a flat vector search, is allocated inside lance-c and is charged to neither the query nor its workload group. It counts toward the BE process memory only, so memory GC, which ranks queries by tracked memory, does not attribute it to the Lance query. lance-c accepts no memory budget, so on BEs with little memory headroom size `lance_io_buffer_size_bytes`, `lance_batch_readahead` and `lance_fragment_readahead` for the number of concurrent Lance scanners. As on branch-4.1 / 4.2; tracked as a follow-up. - Build: macOS links `-framework IOKit`. This is the first time objects of `liblance_c.a` are pulled into `doris_be`, so the binary and the link time grow. With this PR `util/rle_encoding.h` reaches 231 TUs and `util/threadpool.h` 589, the same as master `315c2e00331` alone (limits 275 and 660 since #69717), and `gen_cpp/internal_service.pb.h` 562 (master alone 559, limit 660); the reach budgets are unchanged. Not included: the Lance index metadata scanner (`LANCE_INDEX_ENTRIES`); the FE side, including the rolling-upgrade guard that keeps Lance lazy materialization off while an old BE could decode a version 1 row location as version 0; the regression suites; the rest of #68707 (searching historical versions, tags and branches, rejecting fragment ids missing from the snapshot, naming the snapshot in errors and the profile). The `InitReaderAvgTime` / `GetBlockAvgTime` profile fix that #67005 carried goes in a separate PR. **Results.** | | master | with this PR | |---|---|---| | A `FORMAT_LANCE` query | fails before reading anything | read by `LanceTableReader` in FileScannerV2 | | A Lance row in the second phase of TopN lazy materialization | cannot be represented | a version 1 row location, read back with `take_rows` | | A row location of an unknown version or size in phase two | decoded as version 0 | rejected with an error | | An Arrow DATE32 day outside 0000-01-01..9999-12-31 (ADBC, Arrow stream load, paimon-rust, Lance) | read as a non-null `0000-00-00` | the read fails with `InvalidArgument` | BE unit tests (ASAN, macOS arm64, against master's pinned lance-c), on the head rebased onto master `378404eeb12`: 1029 cases in 54 suites run, 1027 pass and 2 Parquet microbenchmarks skip by design. They cover the Lance suites (107: `LanceTableReader*` 83, `LanceNestedNullTest` 10, `LanceRuntimeFilterHelperTest` 10, `LanceSessionManagerTest` 4); the row-location protocol (`MaterializationSharedStateTest`, `RowIdStorageReaderTest`, and the new `RowIdFetchRequestValidationTest`: count mismatch, unknown version, a Lance version on an internal mapping, an internal row id of 2^32); `FileScannerV2Test` (28); the ORC and Parquet reader suites (`NewOrcReaderTest`, `NewParquetReaderTest`, `ParquetScanTest` and the other `Parquet*` suites), whose row ids now go through `file_local`; and the Arrow / protobuf / DATE serde suites with the new `ArrowParentNullsTest`, plus the Iceberg and Parquet reader suites that decode Arrow dates. Reverting any of the review fixes, the #68813 flag, the #68707 split rule or the #68815 bounds makes its new tests fail. The sixth review round adds 13 cases (`ArrowParentNullsTest` 10, `LanceTableReaderTypeTest` 3) and extends one; with them the Arrow SerDe, Lance, ADBC and `IcebergV2ReaderTest` suites run 317 cases: 310 pass and the 7 ADBC SQLite cases skip without a local SQLite driver. The ninth review round adds `LanceRuntimeFilterHelperTest.KeepsInt64MinFiltersResidualOnly` and `LanceTableReaderFilterTest.KeepsInt64MinRuntimeFiltersOutOfLanceScanner`: both pass with the 106 Lance cases (`Lance*`) on the branch rebased onto `339e087b11f`, and restoring `parse_int64()` in either check makes exactly these two fail. On the head rebased onto master `315c2e00331`, with the per-block mask fix (`42cf4e3e5d1`), 328 cases in 24 suites pass: the Arrow SerDe and validation suites, `ArrowParentNullsTest` (15), `LoadArrowPipeTest` (47), the DATETIMEV2 / TIMESTAMPTZ SerDe suites, `ArrowColumnToDorisColumnTest`, `ArrowBlockConvertorTest`, `AdbcReaderTest`, `RemoteDorisV2ReaderTest`, `PaimonRustTableReaderTest` and the Lance suites. Allocating at the first NULL row of each call again fails the new `ArrowParentNullsTest` case and the four new `LoadArrowPipeTest` block cases, dropping the per-batch mask fails the LIST and STRUCT block cases, and returning the child unmasked fails the new timestamp case, four `ArrowParentNullsTest` cases and `LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents`. ### Release note Add nine `lance_*` BE configs and `lance_session_*` BE metrics. They take effect once the FE can plan Lance scans. Reading an Arrow DATE32 value outside 0000-01-01..9999-12-31 through ADBC, Arrow stream load or paimon-rust now fails instead of returning 0000-00-00. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - `run-be-ut.sh --run --filter=...` with the suites listed under Results: 420 of 420 pass on macOS arm64 (ASAN) on the content before the rebases, and the Lance and Arrow SerDe suites pass again with the nested-offset, zero-field struct, hidden-child, hidden-Decimal256 and compaction-mask fixes (the ADBC SQLite cases skip without a local SQLite driver). Rebased onto master `43f9ceb48d7`: #69668 conflicted in the scanner selection and in `fetch_table_schema`, which now hands every file format to `format::fetch_file_schema()`, so the Lance branch runs ahead of that call; #69646 added a TopN row-group test that reads the version 0 row location fields, which now go through `file_local` (the earlier rebase onto `5caaeb3e3d4` resolved #69614 and #69601 in the scanner selection, the `FileFormat` enum, its test and the reach budget table). The 95 TUs that this PR changes or that master changed since `5caaeb3e3d4` were syntax-checked against the rebased tree with the UT build flags, and CI runs the suites again. Rebased again onto master `8d88f1cc6f7`: the only conflict was the budget table, where #65858 moved the `util/rle_encoding.h` baseline to 828, and the 163 TUs that this PR changes or that master changed since `43f9ceb48d7` pass the same syntax check. Rebased again onto master `339e087b11f`: the only conflict was the budget table again, where #67292 moved the `util/rle_encoding.h` baseline to 834; #69872 added Arrow type checks ahead of the NULL-parent masking in the map and struct readers and merged cleanly. Of the 114 sources that this PR changes or that master changed since `8d88f1cc6f7`, 111 pass the same syntax check. The other three are master's: two new #67292 files under `storage/segment` fail the same way on master alone, since they compile only inside their unity batch, and this PR adds no source to that target; the #69818 benchmark is not in the UT build and includes only `util/simd` headers that this PR does not touch. Reverting any of the review fixes, the #68813 flag, the #68707 split rule or the #68815 bounds makes its new tests fail. Rebased again onto master `378404eeb12`, which removed the legacy file scanner (#69723): the conflicts were in the scanner-selection code and tests that #69723 deleted, so this PR's Lance lines there went with them (including its `LanceQueriesAlwaysUseScannerV2` test), in the profile keys of `materialization_opertor.cpp`, and in the row fetch of `rowid_fetcher.cpp`, whose non-Lance path is now only `FileScannerV2::read_by_rows`; the deleted V1 ORC / Parquet row id tests took this PR's `file_local` rename with them, and the `util/rle_encoding.h` budget commit is dropped because master is no longer near that limit. Of the 100 sources that this PR changes or that master changed since `339e087b11f`, 99 pass the same syntax check; the other, `format_v2/orc/orc_reader.cpp`, fails the same way on master alone. On the rebased head the suites listed under Results run 1029 cases: 1027 pass and 2 microbenchmarks skip. The test-utility fix that `DataTypeDateTimeV2SerDeTest` needed on macOS (#69695) is now on master. Rebased again onto master `315c2e00331`: the only conflict was `TExternalSearchRequest` in `PlanNodes.thrift`, where #70075 had reserved fields 2-5 in comments; the two commits that add the fields replace those comments, and field 6 stays #70075's `paimon_options`. #69717 cut transitive includes from shared headers: of the 160 sources that this PR changes or that master changed since `378404eeb12`, 156 pass the same syntax check; the two `jemalloc_hook` tests have no compile command on macOS, `runtime/memory/jemalloc_hook.cpp` uses glibc aliases that macOS cannot build (#69819), and `format_v2/table_reader.cpp` alone fails the same way on master alone while its unity batch compiles. The full `doris_be_test` builds and links on the rebased head; `ArrowParentNulls*` and `Lance*` (121 cases) pass before the mask fix, and the 328 cases above with it. - `build-support/check-build-hygiene.sh` (on this branch rebased onto master `315c2e00331`), clang-format 16 and `build-support/run-clang-tidy.sh` on the changed files: all pass; on the changed lines clang-tidy (llvm 22) reports only `readability-inconsistent-ifelse-braces` on the `DORIS_CHECK` lines of the masking functions, the macro's expansion that master's existing `DORIS_CHECK`s report too. - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - Behavior changed: - [ ] No. - [x] Yes. `PRequestBlockDesc.row_id` is `uint64`; phase two of TopN lazy materialization rejects row locations of an unknown version or size; `FORMAT_LANCE` scans run on FileScannerV2; an Arrow DATE32 day outside the Doris DATE range fails the read instead of becoming `0000-00-00`. - Does this need documentation? - [x] No. Lance is documented with the FE connector. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Co-authored-by: zhangstar333 <zhangsida@selectdb.com> Co-authored-by: Gabriel <liwenqiang@selectdb.com> Co-authored-by: zhaobo wang <wangzhaobo957@gmail.com> Co-authored-by: jay <qingfengyouchen@163.com> Co-authored-by: FANNG <fanng@apache.org> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: zy-kkk <zhongyk10@gmail.com>
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.
What problem does this PR solve?
Problem Summary:
On glibc < 2.25, dynamic TLS can be allocated by libc and freed by jemalloc, crashing during thread cleanup. Route all seven glibc allocation entry points through the existing jemalloc wrappers, preserve allocator API semantics, and add native interposition tests.
Release note
Fix a BE crash during dynamic TLS cleanup on older glibc systems and keep glibc allocation entry points paired with jemalloc.
Check List (For Author)
Test
./run-be-ut.sh --jemalloc-hook --run -j 2: 12/12 passed on Linux x86_64/glibc 2.32. Full BE, older glibc, ARM64 and customer JNI E2E were not run.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)