Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix](config) for compatibility issue of log dir config #34734

Merged
merged 2 commits into from
May 12, 2024

Conversation

morningman
Copy link
Contributor

Proposed changes

PR #32933 change the config log dir logic, but result result empty sys_log_dir when calling
/rest/v2/manager/node/configuration_info rest API.

This PR make it compatible with old version, return the real log dir as sys_log_dir's value in rest API

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@morningman
Copy link
Contributor Author

run buildall

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Collaborator

@shuke987 shuke987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

PR approved by anyone and no changes requested.

yiguolei
yiguolei previously approved these changes May 11, 2024
@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 11, 2024
Copy link
Contributor

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

@morningman
Copy link
Contributor Author

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label May 11, 2024
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.65% (8985/25201)
Line Coverage: 27.32% (74272/271843)
Region Coverage: 26.56% (38389/144556)
Branch Coverage: 23.37% (19571/83756)
Coverage Report: http://coverage.selectdb-in.cc/coverage/c735b02d5ca717dfc6c7b99d5f71835fd0932336_c735b02d5ca717dfc6c7b99d5f71835fd0932336/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17603	4385	4212	4212
q2	2024	192	199	192
q3	10424	1313	1232	1232
q4	10195	783	843	783
q5	7444	2733	2830	2733
q6	227	133	137	133
q7	1028	604	605	604
q8	9240	2135	2084	2084
q9	10290	6710	6739	6710
q10	9074	3878	3843	3843
q11	452	255	237	237
q12	417	246	229	229
q13	17254	3242	3162	3162
q14	276	224	231	224
q15	526	484	474	474
q16	528	420	406	406
q17	1019	774	638	638
q18	8285	7735	7765	7735
q19	4088	1540	1532	1532
q20	652	331	321	321
q21	5240	4215	4063	4063
q22	375	290	294	290
Total cold run time: 116661 ms
Total hot run time: 41837 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4519	4479	4400	4400
q2	384	275	265	265
q3	3179	2909	2933	2909
q4	1955	1714	1619	1619
q5	5309	5464	5474	5464
q6	215	128	131	128
q7	2333	1968	1934	1934
q8	3245	3467	3374	3374
q9	8572	8675	8739	8675
q10	4099	3861	3738	3738
q11	586	479	495	479
q12	772	596	618	596
q13	16083	3133	3204	3133
q14	314	292	249	249
q15	525	488	481	481
q16	483	430	457	430
q17	1812	1509	1541	1509
q18	7939	7452	7342	7342
q19	1647	1574	1530	1530
q20	1978	1778	1731	1731
q21	11422	4850	4850	4850
q22	603	500	494	494
Total cold run time: 77974 ms
Total hot run time: 55330 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 187802 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 c735b02d5ca717dfc6c7b99d5f71835fd0932336, data reload: false

query1	903	370	358	358
query2	6846	2460	2264	2264
query3	6636	219	221	219
query4	25338	21171	21219	21171
query5	4093	419	419	419
query6	259	186	200	186
query7	4589	297	289	289
query8	236	197	188	188
query9	8495	2440	2405	2405
query10	436	250	269	250
query11	14695	14299	14372	14299
query12	136	89	86	86
query13	1651	381	371	371
query14	9715	8506	8374	8374
query15	241	166	174	166
query16	8023	267	269	267
query17	1721	585	561	561
query18	2008	286	274	274
query19	242	156	159	156
query20	92	89	84	84
query21	197	135	128	128
query22	5039	4820	4765	4765
query23	33896	33500	33623	33500
query24	7369	2941	2948	2941
query25	566	368	382	368
query26	701	157	155	155
query27	2116	321	314	314
query28	5364	2079	2044	2044
query29	852	594	604	594
query30	253	152	150	150
query31	964	745	729	729
query32	86	52	55	52
query33	529	267	247	247
query34	864	490	480	480
query35	770	691	704	691
query36	1078	894	936	894
query37	107	65	65	65
query38	2904	2760	2724	2724
query39	1590	1554	1545	1545
query40	198	123	121	121
query41	42	38	37	37
query42	106	98	99	98
query43	564	554	538	538
query44	1083	722	738	722
query45	270	246	251	246
query46	1071	755	716	716
query47	1955	1873	1898	1873
query48	374	296	299	296
query49	846	392	395	392
query50	773	395	382	382
query51	6886	6718	6758	6718
query52	100	89	97	89
query53	356	301	305	301
query54	540	435	432	432
query55	73	71	70	70
query56	236	219	221	219
query57	1239	1170	1132	1132
query58	213	193	201	193
query59	3356	3080	3221	3080
query60	258	235	234	234
query61	100	86	86	86
query62	626	459	466	459
query63	308	289	290	289
query64	8451	7370	7435	7370
query65	3138	3116	3079	3079
query66	788	338	351	338
query67	15409	14881	14960	14881
query68	4516	546	522	522
query69	475	310	301	301
query70	1195	1144	1147	1144
query71	369	274	270	270
query72	7208	2569	2392	2392
query73	713	332	329	329
query74	6507	6104	6110	6104
query75	3329	2621	2614	2614
query76	2252	1039	945	945
query77	406	263	259	259
query78	10660	10121	10178	10121
query79	2324	520	520	520
query80	1079	425	437	425
query81	515	219	229	219
query82	734	96	93	93
query83	235	164	170	164
query84	252	84	87	84
query85	1323	268	257	257
query86	528	324	285	285
query87	3346	3181	3099	3099
query88	4177	2445	2447	2445
query89	475	377	379	377
query90	2028	188	193	188
query91	124	99	97	97
query92	65	51	49	49
query93	1846	502	498	498
query94	1167	180	187	180
query95	393	306	309	306
query96	597	270	268	268
query97	3147	3001	3003	3001
query98	231	214	217	214
query99	1196	910	868	868
Total cold run time: 274371 ms
Total hot run time: 187802 ms

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 12, 2024
Copy link
Contributor

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

Copy link
Contributor

@xinyiZzz xinyiZzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yiguolei yiguolei merged commit c6b469a into apache:master May 12, 2024
26 of 29 checks passed
yiguolei pushed a commit that referenced this pull request May 12, 2024
* [fix](config) for compatibility issue of log dir config

* 1
ByteYue pushed a commit to ByteYue/doris that referenced this pull request May 15, 2024
* [fix](config) for compatibility issue of log dir config

* 1
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. dev/2.1.3-merged dev/3.0.0-merged p0_b reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants