Skip to content

[improvement](catalog) narrow MaterializedIndex addTablet sync scope#64478

Open
yx-keith wants to merge 1 commit into
apache:masterfrom
yx-keith:narrow-materialized-index-sync
Open

[improvement](catalog) narrow MaterializedIndex addTablet sync scope#64478
yx-keith wants to merge 1 commit into
apache:masterfrom
yx-keith:narrow-materialized-index-sync

Conversation

@yx-keith

@yx-keith yx-keith commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Move the synchronized modifier from the public addTablet / appendTablets methods onto appendTabletsInternal, the only block that actually mutates the per-index list and idToTablets map.

The TabletInvertedIndex.addTablet(...) call in the non-restore path of addTablet does not need to share this lock — it has its own internal synchronization and operates on a separate object. Holding the per-index monitor across that call only serialized unrelated writers without guarding any shared state on this index.

Readers see the new tablets via the same volatile publish in appendTabletsInternal as before, so the existing TOCTOU window between the per-index snapshot and TabletInvertedIndex (handled by the IllegalStateException catch in TabletStatMgr.updateTabletStat) is unchanged in both width and shape.

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #63298

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • 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
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

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

Move the synchronized modifier from the public addTablet / appendTablets
methods onto appendTabletsInternal, the only block that actually mutates
the per-index list and idToTablets map.

The TabletInvertedIndex.addTablet(...) call in the non-restore path of
addTablet does not need to share this lock — it has its own internal
synchronization and operates on a separate object. Holding the per-index
monitor across that call only serialized unrelated writers without
guarding any shared state on this index.

Readers see the new tablets via the same volatile publish in
appendTabletsInternal as before, so the existing TOCTOU window between
the per-index snapshot and TabletInvertedIndex (handled by the
IllegalStateException catch in TabletStatMgr.updateTabletStat) is
unchanged in both width and shape.
@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?

@yx-keith

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	17787	4129	4065	4065
q2	q3	10776	1382	828	828
q4	4698	470	339	339
q5	7659	856	583	583
q6	189	179	138	138
q7	777	822	618	618
q8	10210	1657	1551	1551
q9	6596	4465	4485	4465
q10	6815	1825	1502	1502
q11	446	269	245	245
q12	654	427	289	289
q13	18229	3460	2688	2688
q14	272	256	235	235
q15	q16	820	771	704	704
q17	1423	1132	766	766
q18	6847	5698	5434	5434
q19	1400	1302	1037	1037
q20	548	395	262	262
q21	5943	2761	2552	2552
q22	455	370	320	320
Total cold run time: 102544 ms
Total hot run time: 28621 ms

----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	4928	4716	4629	4629
q2	q3	5112	5186	4699	4699
q4	2092	2242	1405	1405
q5	4889	4771	4726	4726
q6	239	175	134	134
q7	1860	1732	1561	1561
q8	2471	2151	2038	2038
q9	7480	7491	7460	7460
q10	4748	4647	4152	4152
q11	536	384	356	356
q12	729	734	537	537
q13	2982	3439	2831	2831
q14	278	282	257	257
q15	q16	677	693	608	608
q17	1300	1260	1259	1259
q18	7315	7004	6796	6796
q19	1108	1118	1105	1105
q20	2245	2216	1943	1943
q21	5313	4543	4534	4534
q22	519	452	402	402
Total cold run time: 56821 ms
Total hot run time: 51432 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 168410 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 0bb04c57cbcac3c21d949d215b68539985883d6a, data reload: false

query5	4330	621	489	489
query6	469	192	171	171
query7	4954	571	313	313
query8	371	220	202	202
query9	8736	4012	3998	3998
query10	490	313	281	281
query11	5912	2363	2188	2188
query12	160	100	99	99
query13	1309	625	430	430
query14	6413	5355	5064	5064
query14_1	4374	4366	4334	4334
query15	206	195	176	176
query16	1057	455	430	430
query17	1142	710	596	596
query18	2703	483	350	350
query19	211	185	145	145
query20	117	109	106	106
query21	214	137	119	119
query22	13749	13554	13479	13479
query23	17333	16449	16122	16122
query23_1	16327	16234	16271	16234
query24	7531	1773	1290	1290
query24_1	1310	1261	1290	1261
query25	585	459	408	408
query26	1301	327	175	175
query27	2762	574	334	334
query28	4419	2044	2004	2004
query29	1093	637	500	500
query30	322	229	205	205
query31	1125	1093	962	962
query32	110	64	62	62
query33	537	331	261	261
query34	1222	1141	654	654
query35	765	828	659	659
query36	1368	1408	1222	1222
query37	156	102	101	101
query38	3205	3132	3046	3046
query39	924	918	894	894
query39_1	875	903	896	896
query40	212	122	99	99
query41	66	61	60	60
query42	93	96	89	89
query43	314	324	275	275
query44	
query45	189	182	178	178
query46	1066	1208	728	728
query47	2381	2391	2285	2285
query48	386	398	296	296
query49	626	452	341	341
query50	1026	367	258	258
query51	4323	4399	4182	4182
query52	90	85	74	74
query53	251	259	188	188
query54	302	212	194	194
query55	77	78	69	69
query56	215	226	217	217
query57	1429	1402	1362	1362
query58	248	201	212	201
query59	1622	1646	1403	1403
query60	295	258	231	231
query61	152	146	145	145
query62	708	646	590	590
query63	226	186	185	185
query64	2538	785	601	601
query65	
query66	1726	459	381	381
query67	29685	29705	29500	29500
query68	
query69	421	303	261	261
query70	977	991	969	969
query71	297	217	206	206
query72	2939	2682	2282	2282
query73	900	761	444	444
query74	5134	4948	4789	4789
query75	2649	2547	2211	2211
query76	2316	1144	776	776
query77	342	375	287	287
query78	12380	12462	11770	11770
query79	1468	1081	720	720
query80	1328	466	376	376
query81	557	284	254	254
query82	628	162	122	122
query83	326	281	253	253
query84	
query85	947	497	408	408
query86	457	289	274	274
query87	3375	3394	3232	3232
query88	3661	2729	2717	2717
query89	448	377	339	339
query90	1924	182	165	165
query91	171	153	132	132
query92	60	59	57	57
query93	1456	1374	921	921
query94	765	361	314	314
query95	700	461	344	344
query96	1062	766	331	331
query97	2714	2687	2586	2586
query98	208	205	202	202
query99	1153	1176	1026	1026
Total cold run time: 252597 ms
Total hot run time: 168410 ms

@yx-keith

Copy link
Copy Markdown
Contributor Author

run feut

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants