Skip to content

[improvement](be) Reduce transitive dependencies in shared headers - #69717

Merged
yiguolei merged 3 commits into
apache:masterfrom
linrrzqqq:reduce-heavy-header-deps
Nov 3, 2026
Merged

yiguolei merged 3 commits into
apache:masterfrom
linrrzqqq:reduce-heavy-header-deps

Conversation

@linrrzqqq

@linrrzqqq linrrzqqq commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Related PR: #66901

What problem does this PR solve?

Several shared BE headers transitively include protobuf, Thrift, thread-pool, and storage-reader implementations. Translation units that only need declarations must still parse these definitions, increasing compilation work and the rebuild scope when those headers change.

Recent CI builds failed during CMake configuration because the reverse-reach counts for threadpool.h and rle_encoding.h exceeded the build-hygiene limits. This PR audits all 32 header-budget entries and reduces unnecessary dependencies without changing the budgets or exception lists.

The changes:

  • Extract ColumnIteratorOptions into a dedicated header so vexpr_context.h does not require the full column_reader.h dependency chain.
  • Use forward declarations for thread-pool, protobuf, cache, and WorkloadGroup types where complete definitions are unnecessary.
  • Move UniqueId conversions, protobuf methods in numeric SerDe, and selected construction/destruction operations into .cpp files.
  • Extract SyncSizeCallback into a dedicated header to reduce RPC dependencies propagated through the runtime-filter interface.
  • Add direct includes at actual use sites and verify independent compilation.

Object layouts, move semantics, explicit template instantiations, and per-value decoding implementations are preserved. Moving some definitions into .cpp files changes their visibility for inlining in ordinary separate compilation. Query runtime performance has not been measured in this PR.

Dependency reductions

The following counts come from Ninja dependencies after completed builds, covering 1,531 production translation units and 52 generated translation units:

Header TUs Before TUs Now New Limit
util/rle_encoding.h 846 231 250
util/threadpool.h 1,056 585 600
gen_cpp/types.pb.h 1,862 1,225 1,250
gen_cpp/segment_v2.pb.h 1,231 1,156 1,200
gen_cpp/internal_service.pb.h 777 557 600
gen_cpp/FrontendService_types.h 593 293 300
gen_cpp/cloud.pb.h 325 30 50
gen_cpp/BackendService_types.h 195 115 150
gen_cpp/data.pb.h 228 90 100
storage/options.h 502 293 350
io/fs/s3_file_system.h 65 31 50
gen_cpp/Types_types.h 2,078 2,073 2,100
gen_cpp/PaloInternalService_types.h 1,093 1,093 1,100
util/brpc_closure.h 62 35 50
gen_cpp/Status_types.h 37 37 41
runtime/workload_group/workload_group.h 43 38 47

Across the 16 monitored heavy headers, measured header-to-TU dependency pairs decrease from 8,108 to 6,617, an 18.39% reduction. These removals affect 742 distinct translation units. The number of production compilation units remains unchanged.

The transitive project-header closure of dependency.h decreases from 325 to 246 headers, a 24.31% reduction. This uses the hygiene script’s static include graph and is reported separately from the measured Ninja dependency counts.

Query runtime performance

TPC-H SF1000

Date / Version Hot-min (s) PR vs. Master
master 2026-10-08 59.615 -9.93%
master 2026-10-13 60.776 -11.65%
master 2026-10-15 57.021 -5.83%
master 2026-10-20 55.877 -3.90%
master 2026-10-27 65.987 -18.62%
PR #69717 53.697 —

SSB SF1000

Date / Version Hot-min (s) PR vs. Master
master 2026-10-08 11.418 +3.71%
master 2026-10-13 11.749 +0.79%
master 2026-10-15 10.653 +11.16%
master 2026-10-20 10.773 +9.92%
master 2026-10-27 11.471 +3.23%
PR #69717 11.842 —

ClickBench

Configuration: cluster_type=single, ecs.c9i.4xlarge_16c32g, one 500 GB disk, full analyze, PR #69717 release package.

Date / Version Hot-min (s) PR vs. Master
master 2026-10-08 24.717 -1.83%
master 2026-10-13 26.406 -8.11%
master 2026-10-15 25.079 -3.25%
master 2026-10-20 24.629 -1.48%
master 2026-10-27 23.643 +2.63%
PR #69717 single rerun 24.265 —

TPC-DS SF1000 (1 TB)

Comparisons use the PR result from 2026-10-29. The 2026-10-28 rerun is listed separately.

Date / Version Hot-min (s) PR vs. Master
master 2026-10-08 168.926 -7.40%
master 2026-10-13 170.676 -8.35%
master 2026-10-15 163.170 -4.13%
master 2026-10-20 163.926 -4.58%
master 2026-10-27 195.853 -20.13%
PR #69717 2026-10-29 main result 156.423 —

Release note

None

Check List (For Author)

  • Test: Three complete Release builds; standard ASAN BE/FE build and final incremental build; all BE UT sources compiled and linked; 29 selected unit tests passed with -j50 --coverage; 1,024-row service read, aggregate, shuffle join, sort and cast checks; header hygiene and changed-line format checks passed. Clang-tidy covered all 71 files and retained two existing warnings in relocated protobuf methods (function size and signedness).
  • Behavior changed: No
  • Does this need documentation: No

@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?

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

/review

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

run buildall

@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.

No actionable introduced defects found at head 3ee2057df441d6e165e290fd11d4b4fe34157dc2. Reviewed all 71 changed files and relevant callers, implementations, ownership paths and tests. Two scope reviewers and a separate exchange/runtime-filter risk reviewer each returned NO_NEW_VALUABLE_FINDINGS; the main review independently verified their conclusions. The review converged in one round, with no unresolved candidates or unreviewed changed paths.

Critical checkpoint conclusions:

Checkpoint Conclusion
Goal and supporting evidence The changes reduce transitive dependencies through extracted contracts, forward declarations and relocated definitions. The text-only build-hygiene gates pass; compile-time savings were not independently measured.
Scope and clarity The changes stay focused on header dependencies, equivalent definition/initializer moves and direct use-site includes.
Concurrency and locks Exchange queue/dependency signaling, runtime-filter RPC coordination, cancellation and lock order retain their existing behavior. No new critical section or blocking operation is introduced.
Lifecycle and static initialization BlockItem explicitly retains its move operations and passive destruction. Weak callback owners, incomplete-type ownership and TabletManager cache initialization remain valid. No new cross-TU static initialization dependency was found.
Configuration No configuration item or dynamic-update behavior changes.
Compatibility and core correctness Numeric protobuf bodies, UID bit representations, compression choices and ColumnIteratorOptions layout are preserved. Visibility/version, delete-bitmap and error-propagation logic is unchanged. Relocated definitions remain in existing linked targets.
Thrift/protobuf definitions No gensrc field IDs, enum values or requiredness changes.
Parallel paths Checked local/remote exchange, callback success/error/cancellation, specialized numeric/IP/UUID SerDes, cloud/local consumers, scanner v1/v2 and standalone target dependencies. No missing parallel update found.
Conditional checks No new behavioral branch is introduced; existing checks and their explanations are retained. Qualified std::abs handles the same bounded signed subsecond counts.
Test coverage Inspected existing protobuf round-trip, UID, callback, exchange and affected module tests, including error/cancellation cases. Test changes add direct includes or qualify the same RowRanges type. No new functional behavior requires a new expected result.
Test results and fixtures No result fixtures or behavioral test expectations change. No test was executed in this review.
Observability Existing logs, counters and profiling are preserved; no new execution path requires additional instrumentation.
Transactions and persistence No EditLog, persistence, version-publication or failover protocol changes.
Writes, atomicity and crashes No transaction/data-write algorithm changes; inspected ownership and cleanup relocations preserve their prior behavior.
FE-to-BE variables No transmitted variable or request-field changes.
Performance and memory No additional per-value decoding operation, copy, allocation or scan is introduced. Memory ownership/accounting is preserved. Some definitions become out of line; runtime performance was not measured, and no concrete runtime regression was established.
Other dependency obligations All 18 numeric SerDe class instantiations remain paired with extern declarations. Header layering/budgets/PCH and unity-skip checks pass. No additional actionable issue found.

There were no existing inline review threads or confirmed existing P0/P1 findings to carry forward. No additional user focus was supplied.

Validation: ran only bash build-support/check-build-hygiene.sh and static source/diff/include-graph analysis. No builds, test execution, benchmarks, source edits or submodule initialization. Actual compilation/linking, runtime behavior, release ELF compatibility and the PR's reported performance measurements were not independently verified under this static-only contract.

Review converged after 1 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28905 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 3ee2057df441d6e165e290fd11d4b4fe34157dc2, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17594	5211	5087	5087
q2	2162	316	261	261
q3	10210	1412	822	822
q4	4679	463	334	334
q5	7436	797	530	530
q6	184	191	154	154
q7	764	817	609	609
q8	9250	1342	1065	1065
q9	5496	4223	4191	4191
q10	6836	1286	1032	1032
q11	436	246	235	235
q12	642	398	282	282
q13	18101	3603	3067	3067
q14	276	267	249	249
q15	q16	729	719	683	683
q17	1206	759	608	608
q18	7091	6213	6148	6148
q19	1132	971	620	620
q20	390	341	228	228
q21	5720	2538	2432	2432
q22	393	345	268	268
Total cold run time: 100727 ms
Total hot run time: 28905 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5986	5952	6148	5952
q2	702	611	591	591
q3	5615	5862	5395	5395
q4	2029	2161	1585	1585
q5	5860	5869	5638	5638
q6	245	185	144	144
q7	2252	2106	1796	1796
q8	3027	2684	2678	2678
q9	7935	7513	7506	7506
q10	3658	3613	3116	3116
q11	585	413	378	378
q12	671	664	523	523
q13	3329	3680	3041	3041
q14	285	294	279	279
q15	q16	674	672	601	601
q17	7813	7170	6940	6940
q18	12955	12194	12977	12194
q19	920	828	805	805
q20	2138	2138	1929	1929
q21	5610	4610	4726	4610
q22	480	443	406	406
Total cold run time: 72769 ms
Total hot run time: 66107 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151634 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 3ee2057df441d6e165e290fd11d4b4fe34157dc2, data reload: false

query5	4316	574	445	445
query6	424	203	191	191
query7	4803	448	231	231
query8	323	162	154	154
query9	8799	3232	3243	3232
query10	488	301	260	260
query11	6068	3863	3582	3582
query12	153	95	89	89
query13	1265	454	343	343
query14	6153	3932	3670	3670
query14_1	3481	3440	3437	3437
query15	209	204	184	184
query16	945	425	442	425
query17	863	661	554	554
query18	2427	430	311	311
query19	208	174	137	137
query20	93	98	86	86
query21	213	129	107	107
query22	12870	12974	12730	12730
query23	13524	13203	12513	12513
query23_1	12458	12562	12519	12519
query24	7370	862	482	482
query24_1	492	489	491	489
query25	548	423	365	365
query26	1267	261	141	141
query27	2764	447	280	280
query28	4611	2064	2048	2048
query29	1554	627	468	468
query30	304	235	193	193
query31	988	929	826	826
query32	165	109	112	109
query33	541	309	265	265
query34	903	860	522	522
query35	746	752	651	651
query36	812	808	734	734
query37	125	100	85	85
query38	1771	1723	1651	1651
query39	792	790	758	758
query39_1	737	731	764	731
query40	221	117	99	99
query41	74	69	72	69
query42	89	86	88	86
query43	345	362	316	316
query44	1341	718	756	718
query45	179	177	164	164
query46	828	974	585	585
query47	2724	3014	2762	2762
query48	312	316	218	218
query49	579	395	296	296
query50	674	276	210	210
query51	10556	10640	10415	10415
query52	80	79	71	71
query53	190	207	164	164
query54	259	194	177	177
query55	72	73	63	63
query56	233	233	221	221
query57	1659	1623	1578	1578
query58	287	263	258	258
query59	2129	2203	1971	1971
query60	277	246	222	222
query61	145	145	144	144
query62	383	334	285	285
query63	190	157	157	157
query64	2736	965	773	773
query65	3346	3287	3240	3240
query66	1767	422	307	307
query67	19921	20243	20074	20074
query68	3049	1003	633	633
query69	379	295	242	242
query70	853	786	777	777
query71	312	242	223	223
query72	2693	2563	2313	2313
query73	514	543	306	306
query74	4618	4511	4313	4313
query75	2347	2281	1910	1910
query76	2310	1004	587	587
query77	355	384	278	278
query78	9824	9741	9181	9181
query79	1077	870	545	545
query80	1228	416	347	347
query81	607	319	269	269
query82	845	144	105	105
query83	313	200	185	185
query84	310	127	102	102
query85	863	446	374	374
query86	395	236	237	236
query87	2026	1936	1818	1818
query88	3832	2792	2773	2773
query89	337	291	241	241
query90	1870	167	155	155
query91	167	152	130	130
query92	96	88	86	86
query93	1136	1017	591	591
query94	643	322	273	273
query95	582	395	305	305
query96	680	486	234	234
query97	2452	2447	2306	2306
query98	158	159	146	146
query99	754	761	635	635
Total cold run time: 235612 ms
Total hot run time: 151634 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.58 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 3ee2057df441d6e165e290fd11d4b4fe34157dc2, data reload: false

query1	0.00	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.09	0.08
query4	1.61	0.10	0.10
query5	0.26	0.24	0.26
query6	1.17	0.68	0.68
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.38	0.32	0.34
query10	0.58	0.59	0.59
query11	0.20	0.14	0.14
query12	0.18	0.14	0.15
query13	0.49	0.49	0.48
query14	1.02	1.02	1.01
query15	0.63	0.62	0.62
query16	0.36	0.35	0.35
query17	1.14	1.18	1.17
query18	0.19	0.20	0.19
query19	2.11	2.03	2.04
query20	0.02	0.01	0.01
query21	15.45	0.20	0.14
query22	4.94	0.06	0.05
query23	16.28	0.33	0.12
query24	3.05	0.66	0.51
query25	0.12	0.06	0.05
query26	1.52	0.23	0.15
query27	0.04	0.04	0.03
query28	3.49	0.75	0.35
query29	12.48	4.12	3.28
query30	0.28	0.13	0.11
query31	2.77	0.55	0.32
query32	3.22	0.60	0.50
query33	3.22	3.19	3.37
query34	15.41	4.05	3.39
query35	3.36	3.34	3.36
query36	0.56	0.44	0.44
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.16	0.14	0.14
query41	0.09	0.04	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 97.49 s
Total hot run time: 24.58 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 97.01% (195/201) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 69.06% (32289/46755)
Line Coverage 53.76% (351011/652887)
Region Coverage 49.40% (289433/585881)
Branch Coverage 50.70% (135460/267171)

1 similar comment
@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 97.01% (195/201) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 69.06% (32289/46755)
Line Coverage 53.76% (351011/652887)
Region Coverage 49.40% (289433/585881)
Branch Coverage 50.70% (135460/267171)

@linrrzqqq
linrrzqqq force-pushed the reduce-heavy-header-deps branch from 3ee2057 to 4d2d1e4 Compare November 2, 2026 03:02
@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28710 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 4d2d1e4751ec5e55416eb2822020215cd7e8ba74, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17620	5165	5046	5046
q2	2086	330	246	246
q3	10246	1402	818	818
q4	4675	469	328	328
q5	7435	799	527	527
q6	182	185	152	152
q7	741	790	591	591
q8	9258	1351	1043	1043
q9	5532	4208	4209	4208
q10	6822	1291	1050	1050
q11	444	244	230	230
q12	632	403	271	271
q13	18102	3707	3023	3023
q14	275	267	249	249
q15	q16	734	725	668	668
q17	1208	748	588	588
q18	7020	6201	6137	6137
q19	1114	1000	589	589
q20	401	339	226	226
q21	5200	2431	2496	2431
q22	385	322	289	289
Total cold run time: 100112 ms
Total hot run time: 28710 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5970	5908	6060	5908
q2	762	593	589	589
q3	5700	5838	5468	5468
q4	2011	2158	1556	1556
q5	5911	5896	5652	5652
q6	245	193	141	141
q7	2214	2035	1774	1774
q8	2962	2643	2642	2642
q9	8037	7530	7514	7514
q10	3655	3562	3179	3179
q11	576	406	388	388
q12	659	689	509	509
q13	3323	3535	3097	3097
q14	292	292	260	260
q15	q16	674	674	615	615
q17	7695	7032	6904	6904
q18	12996	12206	12869	12206
q19	881	767	766	766
q20	2137	2158	1925	1925
q21	5575	4601	4694	4601
q22	501	443	425	425
Total cold run time: 72776 ms
Total hot run time: 66119 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151921 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 4d2d1e4751ec5e55416eb2822020215cd7e8ba74, data reload: false

query5	4311	597	429	429
query6	421	207	194	194
query7	4799	456	233	233
query8	320	167	153	153
query9	8787	3192	3209	3192
query10	472	297	252	252
query11	6052	3845	3569	3569
query12	147	90	89	89
query13	1265	474	331	331
query14	6148	3901	3688	3688
query14_1	3439	3421	3447	3421
query15	206	198	183	183
query16	940	464	443	443
query17	848	669	548	548
query18	2418	427	317	317
query19	204	170	138	138
query20	86	85	84	84
query21	211	126	107	107
query22	13003	12969	12802	12802
query23	13623	12902	12520	12520
query23_1	12574	12699	12777	12699
query24	7374	822	484	484
query24_1	485	487	479	479
query25	537	426	367	367
query26	1267	268	139	139
query27	2759	437	294	294
query28	4623	2062	2042	2042
query29	1555	600	490	490
query30	301	216	183	183
query31	996	926	810	810
query32	140	89	93	89
query33	516	293	240	240
query34	907	860	507	507
query35	721	739	632	632
query36	827	817	721	721
query37	119	90	77	77
query38	1771	1709	1663	1663
query39	789	783	740	740
query39_1	736	729	739	729
query40	219	116	92	92
query41	66	64	62	62
query42	84	84	80	80
query43	338	357	320	320
query44	1324	696	705	696
query45	185	178	166	166
query46	843	967	557	557
query47	3035	2950	2843	2843
query48	296	322	226	226
query49	572	385	292	292
query50	708	274	206	206
query51	10693	10822	10669	10669
query52	78	79	67	67
query53	187	206	148	148
query54	233	188	179	179
query55	67	65	64	64
query56	224	204	203	203
query57	1662	1623	1561	1561
query58	292	274	263	263
query59	2258	2249	2056	2056
query60	302	243	213	213
query61	146	148	174	148
query62	391	328	298	298
query63	190	157	159	157
query64	2729	956	764	764
query65	3313	3278	3269	3269
query66	1763	419	293	293
query67	20166	20022	19915	19915
query68	3042	1020	621	621
query69	385	273	250	250
query70	847	770	750	750
query71	280	227	201	201
query72	2701	2591	2384	2384
query73	512	529	295	295
query74	4612	4474	4358	4358
query75	2334	2275	1895	1895
query76	2298	1014	580	580
query77	333	368	260	260
query78	9801	9613	9173	9173
query79	1011	829	519	519
query80	1212	407	345	345
query81	587	321	275	275
query82	735	130	104	104
query83	295	199	181	181
query84	304	123	97	97
query85	865	432	391	391
query86	408	247	231	231
query87	1981	1955	1795	1795
query88	3791	2774	2760	2760
query89	327	280	237	237
query90	1913	170	165	165
query91	165	149	124	124
query92	98	89	89	89
query93	1020	988	567	567
query94	680	308	285	285
query95	580	333	320	320
query96	686	480	244	244
query97	2461	2432	2305	2305
query98	159	154	149	149
query99	774	754	662	662
Total cold run time: 235913 ms
Total hot run time: 151921 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.26 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 4d2d1e4751ec5e55416eb2822020215cd7e8ba74, data reload: false

query1	0.00	0.00	0.00
query2	0.09	0.05	0.05
query3	0.26	0.09	0.09
query4	1.60	0.10	0.10
query5	0.23	0.25	0.24
query6	1.16	0.68	0.67
query7	0.04	0.01	0.00
query8	0.05	0.03	0.04
query9	0.40	0.33	0.34
query10	0.61	0.56	0.57
query11	0.22	0.15	0.14
query12	0.19	0.15	0.14
query13	0.50	0.49	0.49
query14	1.04	1.01	1.02
query15	0.62	0.60	0.61
query16	0.34	0.36	0.34
query17	1.18	1.10	1.14
query18	0.20	0.18	0.18
query19	2.07	1.99	2.01
query20	0.02	0.01	0.01
query21	15.50	0.19	0.13
query22	4.86	0.06	0.05
query23	16.28	0.31	0.12
query24	3.12	0.43	0.55
query25	0.14	0.05	0.05
query26	1.15	0.21	0.15
query27	0.06	0.05	0.04
query28	4.06	0.75	0.36
query29	12.53	4.19	3.22
query30	0.29	0.13	0.10
query31	2.77	0.56	0.33
query32	3.23	0.59	0.50
query33	3.17	3.20	3.20
query34	15.38	3.99	3.39
query35	3.38	3.35	3.33
query36	0.57	0.46	0.43
query37	0.09	0.06	0.06
query38	0.05	0.04	0.03
query39	0.03	0.03	0.03
query40	0.17	0.15	0.15
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 97.82 s
Total hot run time: 24.26 s

linrrzqqq added a commit to linrrzqqq/doris that referenced this pull request Nov 2, 2026
…leanup

### What problem does this PR solve?

Related PR: apache#69717

Problem Summary: The include cleanup reduced transitive header dependencies, leaving several budgets above the measured reach. Update the forward closure budget and reverse reach baselines to preserve these reductions, with room for subsequent changes. Retain the existing 10% reverse reach allowance and gate behavior. Set the FrontendService_types.h, data.pb.h, storage/options.h and PaloInternalService_types.h baselines to 350, 120, 350 and 1150 respectively.

### Release note

None

### Check List (For Author)

- Test: build-support/check-build-hygiene.sh; build-support/tests/run.sh; git diff --check
- Behavior changed: Yes, header dependency budget values are updated.
- Does this need documentation: No
### What problem does this PR solve?

Problem Summary: Shared BE headers propagate protobuf, Thrift, cache,
thread-pool and storage-reader definitions into unrelated translation units,
increasing repeated parsing and header rebuild scope.

Introduce dedicated ColumnIteratorOptions and SyncSizeCallback headers,
use forward declarations where complete types are unnecessary, and move
protobuf conversions, number SerDe definitions and lifecycle operations
into their implementation files. Add direct includes at actual use sites
and repair independent compilation of affected production and test sources.
Preserve object layouts, move semantics, explicit template instantiations
and per-value decoding implementations.

Across the 16 monitored heavy headers, measured header-to-TU dependency
pairs decrease from 8,108 to 6,617 (18.39%), affecting 742 distinct TUs.
RLE reach decreases from 537 to 154 TUs and threadpool reach from 607 to 361.
The dependency.h project-header closure decreases from 325 to 246.
Header budgets and exception lists remain unchanged.

Release builds with Clang 21.1.8, -j50, PCH/Unity/ccache disabled measured
636.4 seconds before, 569.0 seconds after and 636.2 seconds on repetition.
Concurrent machine load prevents claiming a stable full-build speedup.
Median production compiler peak RSS changes from 563.5 MiB to 551.5 MiB
and 551.6 MiB; production compilation remains at 1,531 translation units.

### Release note

None

### Check List (For Author)

- Test: Three complete Release builds; standard ASAN BE/FE build and final
  incremental build; all BE UT sources compiled and linked; 29 selected
  unit tests passed with -j50 --coverage; 1,024-row service read, aggregate,
  shuffle join, sort and cast checks; header hygiene and changed-line format
  checks passed. Clang-tidy covered all 71 files and retained two existing
  warnings in relocated protobuf methods (function size and signedness).
- Behavior changed: No
- Does this need documentation: No
…leanup

### What problem does this PR solve?

Related PR: apache#69717

Problem Summary: The include cleanup reduced transitive header dependencies, leaving several budgets above the measured reach. Update the forward closure budget and reverse reach baselines to preserve these reductions, with room for subsequent changes. Retain the existing 10% reverse reach allowance and gate behavior. Set the FrontendService_types.h, data.pb.h, storage/options.h and PaloInternalService_types.h baselines to 350, 120, 350 and 1150 respectively.

### Release note

None

### Check List (For Author)

- Test: build-support/check-build-hygiene.sh; build-support/tests/run.sh; git diff --check
- Behavior changed: Yes, header dependency budget values are updated.
- Does this need documentation: No
@linrrzqqq
linrrzqqq force-pushed the reduce-heavy-header-deps branch from e930737 to dc2a6a3 Compare November 2, 2026 11:46
@linrrzqqq

Copy link
Copy Markdown
Collaborator 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.

Found two P1 build regressions at dc2a6a3dd587865ae137d5cc5a9e85bb2359b7b0: the S3 reader test loses the complete ObjClientHolder definition, and cache reload callers lose CachePath and parse_conf_cache_paths. Both need direct includes at their actual use sites. These are separate dependency cuts and are reported inline. No existing P0/P1 findings apply; the supplied threads and live inline-comment readback were empty.

Reviewed all 69 changed paths in the authoritative aggregate diff, including changed tests, related callers, and local/remote and cloud/shared-nothing paths. No additional user focus was supplied.

Required checkpoint Conclusion
Goal and evidence Header dependencies decrease and the tightened static budgets pass. The two missed consumers prevent the claimed complete independent-compilation cleanup. Existing behavioral tests were inspected, not executed.
Scope and clarity The changes remain focused on dependency removal, declaration extraction, moved definitions, and matching use-site includes.
Concurrency and locks Queue ingress, query dequeue, cancellation/close, and RPC completion retain their existing locks and ordering. No new thread entry, shared-state mutation, or blocking-under-lock operation was introduced.
Lifecycle and static initialization BlockItem retains explicit defaulted moves, member initializers, and deferred callback cleanup; callback/query weak ownership is preserved. Tablet stat-cache construction remains before the sole constructor body. No new cross-TU static initialization dependency was found.
Configuration No runtime configuration added. The changed build-gate thresholds remain static tables with the existing enforcement logic.
Symbols, layout, and rolling compatibility Internal definitions move into already-linked source files; all 18 numeric SerDe extern/explicit families remain paired. Extracted options/callback bodies and protobuf implementations are identical. No wire or storage layout change was found; binary compatibility was not validated from built artifacts.
Thrift/protobuf field IDs No gensrc/thrift or gensrc/proto definitions changed.
Parallel paths Local/remote exchange, cloud/local storage, numeric/date/IP/UUID SerDes, and direct header consumers were traced. The two reported omissions are in dependent consumers.
Conditions and error handling Compression selection and callback/error branches are unchanged relocations. std::abs qualification retains bounded signed subsecond handling. No new silent-failure branch was found.
Functional and negative coverage Inspected protobuf round trips, UID conversions, exchange delayed-callback/decode-error/cancellation tests, runtime-filter failure/expired-owner tests, and changed reader tests. No tests were run. Missing includes must be fixed before affected test targets compile.
Test results No regression output or expected-value files changed; test edits add includes or qualify the same RowRanges type.
Observability Existing counters, timers, logs, and error propagation remain unchanged; no new runtime path needs instrumentation.
Transactions and persistence No commit/replay, rowset-version, delete-bitmap, or persisted-metadata algorithm changed.
Data writes and atomicity No new write algorithm or transaction boundary; cache admission and publication behavior are unchanged by the header cuts.
FE/BE variables No new cross-process variable or sending path.
Performance No added per-value decoding work, copy, or allocation was found. Moving internal methods out of line changes inlining opportunities; runtime performance and PR benchmark claims were not independently validated.
Other risks The header gate's own static graph and seeded self-test source were checked; no additional gate defect was found.

Read-only static analysis passed: 50 header-layering rules, 10 third-party bans, all 16 forward/16 reverse budgets and PCH whitelist; 298 extern declarations across 50 template families; and all four test-included source files' unity exclusions. dependency.h has 248 project headers in its closure, matching the new limit. Gate injection self-tests were inspected only because they mutate source.

This review is static-only: no builds, unit/regression tests, benchmarks, cluster runs, clang-tidy runs, or release-artifact/glibc ABI audit were performed. No repository source files were changed.

All assigned reviewers returned NO_NEW_VALUABLE_FINDINGS in round 2. All candidates and initial risk items have explicit dispositions, with no unresolved static question or uncovered changed file.

Review converged after 2 round(s).

Comment thread be/src/io/fs/s3_file_reader.h Resolved
### What problem does this PR solve?

Related PR: apache#69717

Problem Summary: Header dependency reductions left the S3 reader test and file-cache consumers without complete type definitions needed for object construction and member access. Include s3_file_system.h directly in the S3 reader test, storage/options.h in the file-cache HTTP action and microbench, and the rate limiter and FDCache definitions in the microbench. This restores independent compilation of the BE_TEST and enabled microbench paths.

### Release note

None

### Check List (For Author)

- Test: Standard ASAN BE/FE build; BE UT target build and 10 S3FileReaderTest/FileCacheActionTest cases with -j50 --coverage; independent compilation without PCH or Unity; build-hygiene; changed-line clang-format; clang-tidy; git diff --check. Coverage rendering reported 20 functions with mismatched profile data.
- Behavior changed: No
- Does this need documentation: No
@linrrzqqq

Copy link
Copy Markdown
Collaborator 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.

No new actionable findings in the 72 changed paths at 09c0f1a. The two previous P1 findings are fixed: 4442228286 by the S3 test's direct holder-definition include, and 4442228578 by direct storage-options includes in both cache callers. Neither remains blocking; neither is reposted.

Critical checkpoint conclusions:

  • Goal and evidence: dependency isolation is implemented through declaration-only interfaces and moved definitions. Static comparison confirms unchanged numeric protobuf methods, callback, iterator-options structure and compression selector; existing relevant tests were inspected.
  • Scope and reuse: the changes remain focused on dependency reduction, direct consumer includes and budget updates, reusing existing implementations.
  • Concurrency: exchange task/receiver threads and RPC callbacks retain their existing queue lock, dependency signaling and callback-release paths. Tablet-stat snapshot/publication still uses _tablet_stat_cache_mutex; no new lock work or lock ordering is introduced.
  • Lifecycle and memory: BlockItem retains defaulted noexcept moves and out-of-line destruction; runtime-filter callback/query/wrapper ownership remains weak where required. Cache initialization preserves member order. No new allocation lifetime, accounting, COW/null-handling or cross-TU static-initialization change was found.
  • Configuration: no configuration item is added or changed; dynamic behavior is unaffected.
  • Compatibility: protobuf bodies, layouts and conversions are preserved; all 18 numeric SerDe extern/explicit instantiation families pair. Moved functions remain in their owning build targets. No new platform API or library dependency is introduced. Compile/link and final-artifact ABI validation were outside this static review.
  • Thrift/protobuf field IDs: no definitions under gensrc are changed; generated-header dependencies and forward declarations were inspected.
  • Parallel paths: production, unit-test, cloud/local and optional tool consumers were inspected, including the previously failing BE_TEST and microbench callers.
  • Conditional behavior: no new runtime condition is introduced. Relocated failure/status branches retain the existing error propagation; std::abs uses the standard integer overload for bounded subsecond values.
  • Test coverage: existing UID, SerDe, exchange cleanup, runtime-filter callback and reader tests were inspected. This refactor adds dependency fixes and namespace qualifications to tests; no new functional test behavior is claimed.
  • Expected results: no regression result file or expected output changes; native decoder ranges/assertions are preserved.
  • Observability: logging, metrics, profile counters and error context retain their existing behavior; no new operational path requires instrumentation.
  • Transactions and persistence: no transaction, replay, visible-version, delete-bitmap or persisted-format behavior changes.
  • Writes and crash handling: data-write algorithms, atomicity boundaries, cleanup and recovery are unchanged by these definition/include moves.
  • FE/BE variables: no new transmitted variable or sending path is introduced.
  • Performance: dependency isolation supports the compile-cost goal, and budget changes are explicitly explained in commit dc2a6a3. No added per-value work or allocation was found; runtime effects of out-of-line calls were not measured here.
  • Other risks: downstream complete-type requirements, generated namespaces, constructor/destructor visibility, PCH/unity independence and budget/self-test assumptions were reviewed.

The read-only header-dependency checker passed all 50 layering rules, 10 third-party bans, 16 forward and 16 reverse budgets, and the PCH whitelist. No builds, unit/regression tests, benchmarks or mutating hygiene self-tests were run. No additional user focus was supplied. The review converged in round 1: all three reviewers returned NO_NEW_VALUABLE_FINDINGS, all changed paths and risk items are covered, and no unresolved candidate remains within this static scope.

Review converged after 1 round(s).

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28813 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 09c0f1a768535c3cf79c0e48d3eb0351bfbaf23e, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17595	5249	5226	5226
q2	2116	340	251	251
q3	10222	1390	841	841
q4	4675	467	338	338
q5	7445	809	525	525
q6	186	185	158	158
q7	747	805	599	599
q8	9261	1353	1071	1071
q9	5493	4247	4246	4246
q10	6826	1306	1026	1026
q11	447	247	235	235
q12	641	409	286	286
q13	18087	3642	3056	3056
q14	285	274	256	256
q15	q16	765	728	671	671
q17	1196	744	585	585
q18	7034	6182	6115	6115
q19	1117	966	602	602
q20	388	351	231	231
q21	5620	2234	2308	2234
q22	360	307	261	261
Total cold run time: 100506 ms
Total hot run time: 28813 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5713	5742	5759	5742
q2	697	557	561	557
q3	5213	5432	4938	4938
q4	2058	2170	1615	1615
q5	5205	5158	5830	5158
q6	253	211	170	170
q7	2352	2100	1889	1889
q8	3165	2998	2745	2745
q9	8022	8065	7902	7902
q10	3767	3646	3295	3295
q11	618	490	430	430
q12	679	691	521	521
q13	3746	3838	3308	3308
q14	326	349	293	293
q15	q16	704	733	664	664
q17	8531	7686	7077	7077
q18	12926	12165	12926	12165
q19	938	807	799	799
q20	2161	2156	1955	1955
q21	6028	4941	4914	4914
q22	512	449	406	406
Total cold run time: 73614 ms
Total hot run time: 66543 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 152070 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 09c0f1a768535c3cf79c0e48d3eb0351bfbaf23e, data reload: false

query5	4313	556	450	450
query6	421	217	199	199
query7	4813	472	238	238
query8	320	172	157	157
query9	8768	3221	3202	3202
query10	474	325	267	267
query11	5914	3896	3623	3623
query12	146	94	87	87
query13	1251	450	339	339
query14	6089	3988	3760	3760
query14_1	3500	3458	3466	3458
query15	215	205	184	184
query16	947	436	402	402
query17	870	661	544	544
query18	2420	422	311	311
query19	194	171	139	139
query20	90	87	91	87
query21	212	131	110	110
query22	13140	13115	13052	13052
query23	13547	12918	12535	12535
query23_1	12621	12476	12485	12476
query24	7339	830	464	464
query24_1	482	480	482	480
query25	530	402	358	358
query26	1261	260	137	137
query27	2756	438	276	276
query28	4621	2036	2030	2030
query29	1557	590	444	444
query30	305	211	185	185
query31	1009	935	823	823
query32	138	92	88	88
query33	520	301	231	231
query34	920	880	517	517
query35	748	751	637	637
query36	812	815	730	730
query37	129	103	85	85
query38	1793	1728	1666	1666
query39	820	786	742	742
query39_1	738	731	735	731
query40	212	115	100	100
query41	69	67	67	67
query42	86	91	81	81
query43	361	367	323	323
query44	1350	712	712	712
query45	181	178	168	168
query46	842	971	586	586
query47	2931	2902	2679	2679
query48	306	315	237	237
query49	597	391	290	290
query50	698	290	214	214
query51	10353	10522	10770	10522
query52	78	87	75	75
query53	189	212	157	157
query54	239	213	203	203
query55	75	68	65	65
query56	237	223	212	212
query57	1677	1633	1578	1578
query58	296	285	295	285
query59	2166	2122	1990	1990
query60	298	248	226	226
query61	148	143	142	142
query62	390	346	311	311
query63	194	160	166	160
query64	2745	927	752	752
query65	3336	3303	3309	3303
query66	1776	419	308	308
query67	20145	20218	19934	19934
query68	2948	1024	621	621
query69	382	299	279	279
query70	869	812	801	801
query71	292	236	210	210
query72	2724	2605	2278	2278
query73	521	540	303	303
query74	4654	4507	4312	4312
query75	2340	2299	1909	1909
query76	2315	1055	618	618
query77	352	390	290	290
query78	9789	9725	9165	9165
query79	911	877	542	542
query80	483	407	327	327
query81	523	338	285	285
query82	229	136	103	103
query83	299	197	187	187
query84	309	119	106	106
query85	838	408	384	384
query86	314	246	228	228
query87	1988	1950	1820	1820
query88	3805	2832	2788	2788
query89	322	281	251	251
query90	1847	178	174	174
query91	159	155	135	135
query92	102	89	90	89
query93	1008	996	591	591
query94	435	328	288	288
query95	581	331	364	331
query96	688	514	243	243
query97	2427	2416	2299	2299
query98	166	154	156	154
query99	765	786	639	639
Total cold run time: 233931 ms
Total hot run time: 152070 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.37 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 09c0f1a768535c3cf79c0e48d3eb0351bfbaf23e, data reload: false

query1	0.01	0.01	0.01
query2	0.10	0.05	0.05
query3	0.26	0.09	0.09
query4	1.60	0.10	0.11
query5	0.23	0.26	0.24
query6	1.17	0.68	0.67
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.41	0.35	0.34
query10	0.58	0.60	0.58
query11	0.21	0.15	0.14
query12	0.18	0.14	0.14
query13	0.50	0.49	0.50
query14	1.01	1.02	1.01
query15	0.63	0.62	0.60
query16	0.36	0.36	0.35
query17	1.13	1.18	1.10
query18	0.20	0.20	0.19
query19	2.14	1.99	1.98
query20	0.01	0.01	0.01
query21	15.42	0.22	0.13
query22	4.81	0.06	0.05
query23	16.27	0.33	0.13
query24	2.86	0.51	0.42
query25	0.12	0.05	0.04
query26	0.92	0.21	0.16
query27	0.05	0.03	0.05
query28	3.49	0.79	0.37
query29	12.53	4.20	3.27
query30	0.30	0.13	0.10
query31	2.77	0.56	0.33
query32	3.22	0.59	0.52
query33	3.16	3.25	3.15
query34	15.41	4.03	3.39
query35	3.36	3.36	3.33
query36	0.57	0.45	0.45
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.19	0.16	0.15
query41	0.08	0.03	0.03
query42	0.04	0.02	0.02
query43	0.04	0.03	0.04
Total cold run time: 96.62 s
Total hot run time: 24.37 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 97.01% (195/201) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.70% (35894/46197)
Line Coverage 63.05% (403729/640364)
Region Coverage 59.65% (341750/572909)
Branch Coverage 60.75% (158134/260297)

@linrrzqqq
linrrzqqq marked this pull request as ready for review November 3, 2026 01:45
@yiguolei
yiguolei merged commit 7acfd9b into apache:master Nov 3, 2026
45 checks passed
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@linrrzqqq
linrrzqqq deleted the reduce-heavy-header-deps branch November 3, 2026 07:09
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 3, 2026
### What problem does this PR solve?

Issue Number: close apache#69884

Related PR: apache#69904

Problem Summary: Merging master 7acfd9b left two Delta gaps. First,
apache#69717 replaced table_reader.h's full ShardedKVCache definition with a
forward declaration, but deltalake_reader_test.cpp constructs a
ShardedKVCache and relied on the transitive include. Include
format/format_common.h directly, as hudi_reader_test.cpp does. Second,
apache#69300 made Parquet readers in one scan share their profile counters
through TableReader's parquet profile cache. Hudi, Iceberg and the base
reader pass that cache, but DeltaReader still built its ParquetReader
without it, so every Delta file created its own set of counters. Pass
the cache, and keep binary-UUID preservation disabled as before.

### Release note

None

### Check List (For Author)

- Test: No local build. The merge was conflict-free. Both changes follow
  the matching master code. COMPILE and BE UT are left to CI.
- Behavior changed: No. Delta scan profiles reuse Parquet counters across files, as other formats do.
- Does this need documentation: No
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 5, 2026
### What problem does this PR solve?

Issue Number: close apache#69884

Related PR: apache#69904

Problem Summary: Merging master 7acfd9b left two Delta gaps. First,
apache#69717 replaced table_reader.h's full ShardedKVCache definition with a
forward declaration, but deltalake_reader_test.cpp constructs a
ShardedKVCache and relied on the transitive include. Include
format/format_common.h directly, as hudi_reader_test.cpp does. Second,
apache#69300 made Parquet readers in one scan share their profile counters
through TableReader's parquet profile cache. Hudi, Iceberg and the base
reader pass that cache, but DeltaReader still built its ParquetReader
without it, so every Delta file created its own set of counters. Pass
the cache, and keep binary-UUID preservation disabled as before.

### Release note

None

### Check List (For Author)

- Test: No local build. The merge was conflict-free. Both changes follow
  the matching master code. COMPILE and BE UT are left to CI.
- Behavior changed: No. Delta scan profiles reuse Parquet counters across files, as other formats do.
- Does this need documentation: No
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 <[email protected]>
Co-authored-by: Gabriel <[email protected]>
Co-authored-by: zhaobo wang <[email protected]>
Co-authored-by: jay <[email protected]>
Co-authored-by: FANNG <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: zy-kkk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants