Repository navigation
[improvement](lance) Upgrade prewarm dependency and remove install guard - #68765
Conversation
### What problem does this PR solve? Related PR: apache#68698 Related issue: apache#68692 (dependency preparation only) Port apache#68698 to master. Upgrade lance-c from `cd63420bfbe27f6f0a1edcc873b9191af7d52852` to `98468344bc9d56aa7ed4a4192e844afa62b34ba1`, including the upstream synchronous index-prewarm C/C++ APIs. Refresh the Foyer patch context while preserving its implementation payload and verify that patch application retains the prewarm APIs and shared-session regression. Remove the Lance-specific installation fingerprint helper, its test harness, and their build/CI calls. Existing installations are no longer rejected or rebuilt because their Lance revision fingerprint differs; unavailable APIs are reported through normal compilation or linking. Preserve master's unrelated build and CI changes. This only prepares the dependency; it does not add a SQL prewarm interface or complete the parent issue. ### Validation - Verified the archive checksum and unchanged Foyer implementation payload. - Passed `lance-prefilter-patch-test.sh`, covering fresh extraction, cached-source reuse, re-extraction, stale patch markers, invalid-patch rejection, macOS definitions, and prewarm API/regression preservation. - Passed `download-thirdparty-fallback-test.sh`, `juicefs-default-mirror-test.sh`, `download-thirdparty-md5-test.sh`, `azure-vcpkg-retry-test.sh`, and `adbc-jni-config-test.sh`. - Passed affected shell syntax checks, workflow YAML parsing, and `git diff --check`. No Doris FE/BE source files change. Full compilation and runtime suites were not run locally; PR CI will validate the build. ### Release note Prepare the Lance dependency for index prewarm and remove Lance-specific installation fingerprint checks. No new SQL interface is exposed. ### Check List (For Author) - Test: Existing dependency/script regression tests and manual validation described above. - Behavior changed: Yes; upgrade lance-c and remove Lance revision fingerprint checks. - Does this need documentation: No new SQL interface. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label (cherry picked from commit ebf6472)
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review complete for PR #68765 at head 4b5d6e9d4a64ed295ed515f2053b0778585833df, after two convergence rounds and a final sweep of all eight changed files. Two P2 installation failures are reported inline. No existing inline comments or P0/P1 blockers were found. Complete older Lance installations are intentionally reusable under this PR's stated contract; no current Doris prewarm caller establishes a separate current-head failure from that choice.
Critical checkpoints:
- Goal and proof: the new pin contains the synchronous C/C++ prewarm bindings; the archive checksum and Foyer patch preimage hashes match the pinned source. The added shell assertions check source text and the patch marker. They do not prove runtime behavior, and this review did not run builds or tests.
- Scope and reuse: changes stay within dependency pin, install scripts, patch context, tests, and CI. The patch's Foyer implementation payload is unchanged. The two install-boundary failures need repair.
- Concurrency and lifecycle: no Doris runtime threads or locks change. Package builds run in isolated subshells; interruption while publishing Lance headers and archive is the lifecycle failure in the first inline comment. No cross-translation-unit initialization change appears.
- Configuration and compatibility: no new runtime configuration or dynamic update is added. The new dependency API is not yet called by Doris, and no FE/BE protocol or persisted format changes. Both Linux and macOS use the same affected install gate.
- Conditions and error handling: the Lance file-existence test can accept an incomplete pair; the external-source preflight can erase the installed prefix before the builder reports a missing mandatory input. These are the two inline findings; no other ignored error path was substantiated.
- Tests and results: upstream API tests and the local source/patch test were inspected, while the old negative installation test and its CI step are deleted. No test or build was executed in this review, and no generated test results changed. Author-reported validation is not independent execution evidence here.
- Observability and performance: no Doris runtime logging, metrics, or hot path changes were introduced. The builder reports missing inputs, but the prefix has already been removed in the second finding. No separate performance defect was found.
- Transactions, persistence, data writes, and FE/BE variables: no Doris transaction, EditLog, storage write, or transmitted-variable path changes. The relevant file writes are third-party installation artifacts covered by the inline findings.
- User focus: no additional focus points were supplied.
Review completion: all candidate concerns were accepted as the two comments, deduplicated, or dismissed with code evidence; the round-two normal and risk-focused reviews found no new valuable findings.
### What problem does this PR solve? Issue Number: None Related PR: apache#68357, apache#68765 Problem Summary: CI merges this branch with master before building. Sync master d437841 so local validation includes the current persistence and file-cache tests and the upstream Lance install-guard removal that restores compatibility with prebuilt compilation images. The merge has no conflict resolutions. ### Release note None ### Check List (For Author) - Test: No need to test this merge separately; targeted validation follows with the CI test fixes. - Behavior changed: No additional behavior beyond upstream master - Does this need documentation: No
What problem does this PR solve?
Related PR: #68698
Related issue: #68692 (dependency preparation only)
Port #68698 to master. Upgrade lance-c from
cd63420bfbe27f6f0a1edcc873b9191af7d52852to98468344bc9d56aa7ed4a4192e844afa62b34ba1, including the upstream synchronous index-prewarm C/C++ APIs. Refresh the Foyer patch context while preserving its implementation payload and verify that patch application retains the prewarm APIs and shared-session regression.Remove the Lance-specific installation fingerprint helper, its test harness, and their build/CI calls. Existing installations are no longer rejected or rebuilt because their Lance revision fingerprint differs; unavailable APIs are reported through normal compilation or linking. Preserve master's unrelated build and CI changes.
This only prepares the dependency; it does not add a SQL prewarm interface or complete the parent issue.
Validation
lance-prefilter-patch-test.sh, covering fresh extraction, cached-source reuse, re-extraction, stale patch markers, invalid-patch rejection, macOS definitions, and prewarm API/regression preservation.download-thirdparty-fallback-test.sh,juicefs-default-mirror-test.sh,download-thirdparty-md5-test.sh,azure-vcpkg-retry-test.sh, andadbc-jni-config-test.sh.git diff --check.No Doris FE/BE source files change. Full compilation and runtime suites were not run locally; PR CI will validate the build.
Release note
Prepare the Lance dependency for index prewarm and remove Lance-specific installation fingerprint checks. No new SQL interface is exposed.
Check List (For Author)
Check List (For Reviewer who merge this PR)