Skip to content

[fix](process list) return unexpected result in FULL mode#64631

Open
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix-show-processlist
Open

[fix](process list) return unexpected result in FULL mode#64631
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix-show-processlist

Conversation

@morrySnow

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Related PR: #25052 #50939

Problem Summary:

2 issues:

  • self fe check is not stable
  • rpc privilege check should use ADMIN instead of GRANT

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

2 issues:
- self fe check is not stable
- rpc privilege check should use ADMIN instead of GRANT

related PR: apache#25052 apache#50939
@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?

@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow

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

Automated review summary for PR #64631:

I did not find blocking issues in the changed code.

Critical checkpoint conclusions:

  • Goal/test proof: The PR changes FE self-identification from host comparison to nodeName in the process-list/frontend metadata paths, and aligns the RPC process-list privilege check with the local ADMIN check. The changed code matches that goal. I did not see a new dedicated test, so coverage appears to rely on existing process-list/frontend behavior and CI.
  • Scope/focus: The GitHub PR diff is limited to FrontendsProcNode and ConnectPoolMgr, and the change is small and focused.
  • Concurrency/locking: No new shared mutable state, locking, or lock-order changes. Existing concurrent connection-map iteration behavior is unchanged.
  • Lifecycle/static initialization: No new lifecycle or static-initialization concerns.
  • Configuration/compatibility/protocol: No new config or thrift fields. The RPC behavior change is limited to the authorization predicate.
  • Parallel paths: The touched local and RPC process-list authorization paths are now consistent on ADMIN; the touched FE self checks now use stable node identity.
  • Tests/results: No test files changed and I did not run local tests during this review.
  • Observability: No additional logging or metrics appear necessary for this narrow change.
  • Transactions/persistence/data writes/FE-BE variable passing: Not applicable.
  • Performance: No material performance impact; the code still performs the same list iteration/filtering.
  • User focus: No additional user-provided review focus points were present.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 0.00% (0/7) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 42.86% (3/7) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17635	4075	4044	4044
q2	2017	314	203	203
q3	10335	1452	835	835
q4	4684	472	351	351
q5	7533	862	570	570
q6	185	171	138	138
q7	786	831	641	641
q8	9342	1582	1609	1582
q9	5854	4485	4483	4483
q10	6823	1837	1566	1566
q11	440	281	253	253
q12	629	418	310	310
q13	18162	3416	2798	2798
q14	268	265	249	249
q15	q16	795	803	722	722
q17	946	916	980	916
q18	6874	5728	5687	5687
q19	1304	1243	1045	1045
q20	517	391	269	269
q21	5975	2741	2398	2398
q22	425	371	315	315
Total cold run time: 101529 ms
Total hot run time: 29375 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4351	4242	4249	4242
q2	343	388	226	226
q3	4562	5004	4394	4394
q4	2057	2166	1400	1400
q5	4419	4315	4299	4299
q6	232	182	135	135
q7	1744	1608	2206	1608
q8	2602	2282	2270	2270
q9	8155	8458	8011	8011
q10	4791	4766	4308	4308
q11	579	421	381	381
q12	779	759	558	558
q13	3294	3741	2983	2983
q14	289	304	284	284
q15	q16	721	769	654	654
q17	1351	1344	1338	1338
q18	8016	7570	7269	7269
q19	1175	1148	1098	1098
q20	2201	2207	1979	1979
q21	5317	4647	4500	4500
q22	519	470	409	409
Total cold run time: 57497 ms
Total hot run time: 52346 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 175366 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 4b04640d3c9935b861623006768f588f3625178a, data reload: false

query5	4338	627	486	486
query6	464	196	178	178
query7	4946	551	299	299
query8	353	213	210	210
query9	8736	4043	4011	4011
query10	436	316	248	248
query11	5955	2346	2155	2155
query12	158	102	99	99
query13	1286	610	411	411
query14	6338	5414	5048	5048
query14_1	4379	4380	4344	4344
query15	206	198	175	175
query16	990	464	451	451
query17	964	718	590	590
query18	2479	500	355	355
query19	205	197	149	149
query20	112	108	105	105
query21	222	141	119	119
query22	13634	13523	13353	13353
query23	17489	16597	16232	16232
query23_1	16248	16273	16253	16253
query24	7456	1765	1313	1313
query24_1	1324	1301	1343	1301
query25	559	467	404	404
query26	1305	321	175	175
query27	2754	601	356	356
query28	4465	2045	2053	2045
query29	1104	630	512	512
query30	317	234	203	203
query31	1116	1076	963	963
query32	117	63	59	59
query33	535	328	293	293
query34	1201	1173	663	663
query35	755	773	670	670
query36	1390	1384	1237	1237
query37	155	98	92	92
query38	3191	3139	3097	3097
query39	945	918	906	906
query39_1	864	875	875	875
query40	220	120	101	101
query41	63	66	63	63
query42	93	93	91	91
query43	320	339	291	291
query44	1425	790	788	788
query45	194	181	167	167
query46	1102	1209	741	741
query47	2395	2338	2169	2169
query48	416	397	289	289
query49	618	465	353	353
query50	998	366	265	265
query51	4370	4286	4214	4214
query52	89	87	76	76
query53	251	263	190	190
query54	263	217	212	212
query55	77	75	69	69
query56	220	220	205	205
query57	1441	1420	1304	1304
query58	241	205	213	205
query59	1595	1664	1458	1458
query60	289	235	226	226
query61	155	149	147	147
query62	702	640	589	589
query63	223	202	192	192
query64	2504	750	575	575
query65	4844	4793	4768	4768
query66	1790	452	328	328
query67	29726	29626	29547	29547
query68	3229	1512	931	931
query69	401	313	263	263
query70	1079	953	956	953
query71	291	235	220	220
query72	2967	2655	2358	2358
query73	864	854	455	455
query74	5110	5006	4762	4762
query75	2626	2599	2224	2224
query76	2334	1163	789	789
query77	351	385	291	291
query78	12438	12631	11823	11823
query79	1384	1172	771	771
query80	1288	469	385	385
query81	525	279	238	238
query82	850	157	120	120
query83	343	276	243	243
query84	316	143	111	111
query85	925	490	409	409
query86	438	302	286	286
query87	3394	3327	3188	3188
query88	3708	2827	2774	2774
query89	434	373	339	339
query90	2005	179	167	167
query91	177	163	125	125
query92	64	58	57	57
query93	1555	1488	917	917
query94	718	367	261	261
query95	677	450	338	338
query96	1055	807	355	355
query97	2720	2697	2543	2543
query98	217	202	200	200
query99	1171	1166	1040	1040
Total cold run time: 262224 ms
Total hot run time: 175366 ms

@hello-stephen

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

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.26	0.14	0.14
query4	1.62	0.14	0.14
query5	0.25	0.23	0.22
query6	1.26	1.10	1.11
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.36	0.31	0.33
query10	0.59	0.60	0.60
query11	0.20	0.15	0.15
query12	0.19	0.15	0.15
query13	0.50	0.49	0.48
query14	1.02	1.00	1.01
query15	0.63	0.59	0.59
query16	0.31	0.33	0.32
query17	1.13	1.11	1.10
query18	0.24	0.23	0.22
query19	2.02	1.88	1.95
query20	0.02	0.02	0.01
query21	15.42	0.22	0.14
query22	4.73	0.05	0.06
query23	16.24	0.32	0.12
query24	2.92	0.45	0.32
query25	0.13	0.05	0.05
query26	0.73	0.21	0.14
query27	0.07	0.04	0.04
query28	3.48	1.01	0.57
query29	12.52	4.46	3.50
query30	0.28	0.16	0.15
query31	2.78	0.62	0.32
query32	3.22	0.61	0.49
query33	3.20	3.14	3.29
query34	15.53	4.25	3.47
query35	3.45	3.49	3.49
query36	0.55	0.44	0.44
query37	0.10	0.06	0.07
query38	0.05	0.04	0.03
query39	0.04	0.02	0.02
query40	0.18	0.16	0.14
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.04
Total cold run time: 96.58 s
Total hot run time: 25.28 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 42.86% (3/7) 🎉
Increment coverage report
Complete coverage report

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants