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](brpc) check failed socket before SetConnected #32790

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

kaijchen
Copy link
Contributor

@kaijchen kaijchen commented Mar 25, 2024

Proposed changes

If socket is failed, Stream::_host_socket will be nullptr and cause coredump in SetConnected.
So we have to check failed socket before calling SetConnected.

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org 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.

@kaijchen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17647	4248	4137	4137
q2	2122	161	151	151
q3	10732	1153	1222	1153
q4	10381	811	767	767
q5	7451	3026	3006	3006
q6	206	125	125	125
q7	1044	595	584	584
q8	9335	1998	2045	1998
q9	7396	6732	6631	6631
q10	8434	3472	3626	3472
q11	441	220	226	220
q12	366	210	197	197
q13	17810	2899	2881	2881
q14	235	209	202	202
q15	509	461	450	450
q16	487	380	374	374
q17	967	541	628	541
q18	7268	6498	6416	6416
q19	3879	1412	1479	1412
q20	543	254	251	251
q21	3585	2928	2836	2836
q22	353	291	297	291
Total cold run time: 111191 ms
Total hot run time: 38095 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4175	4141	4123	4123
q2	328	240	240	240
q3	2999	2875	2844	2844
q4	1872	1561	1564	1561
q5	5308	5338	5318	5318
q6	200	115	120	115
q7	2252	1870	1867	1867
q8	3200	3337	3309	3309
q9	8704	8727	8698	8698
q10	3844	3774	3811	3774
q11	541	441	453	441
q12	701	560	554	554
q13	16914	2813	2844	2813
q14	278	245	264	245
q15	503	457	452	452
q16	459	410	437	410
q17	1757	1502	1491	1491
q18	7527	7400	7219	7219
q19	1625	1526	1513	1513
q20	1901	1725	1727	1725
q21	4913	4841	4810	4810
q22	521	445	429	429
Total cold run time: 70522 ms
Total hot run time: 53951 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.26% (8733/24769)
Line Coverage: 27.06% (71523/264338)
Region Coverage: 26.29% (37107/141125)
Branch Coverage: 23.19% (18976/81836)
Coverage Report: http://coverage.selectdb-in.cc/coverage/d8079226bddfa2fc4b39a211e12d24767e58dd54_d8079226bddfa2fc4b39a211e12d24767e58dd54/report/index.html

Copy link
Contributor

@liaoxin01 liaoxin01 left a comment

Choose a reason for hiding this comment

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

LGTM

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

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

Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

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

query1	945	374	359	359
query2	6556	1900	1949	1900
query3	6706	214	215	214
query4	31447	21257	21374	21257
query5	4372	398	406	398
query6	290	174	196	174
query7	4618	295	288	288
query8	224	170	170	170
query9	9268	2291	2288	2288
query10	559	243	281	243
query11	17189	14214	14173	14173
query12	142	93	87	87
query13	1633	430	440	430
query14	12310	11544	11667	11544
query15	308	204	187	187
query16	8163	263	259	259
query17	1943	580	553	553
query18	2108	290	285	285
query19	313	161	158	158
query20	97	88	91	88
query21	203	130	130	130
query22	4980	4809	4790	4790
query23	33445	32540	32675	32540
query24	11765	2868	2859	2859
query25	634	360	373	360
query26	1768	150	157	150
query27	3015	356	356	356
query28	7868	1864	1852	1852
query29	1019	651	624	624
query30	303	149	145	145
query31	978	725	729	725
query32	93	55	54	54
query33	752	241	246	241
query34	1103	490	488	488
query35	819	611	599	599
query36	1010	914	874	874
query37	266	75	80	75
query38	3607	3456	3446	3446
query39	1467	1466	1424	1424
query40	299	113	109	109
query41	52	50	46	46
query42	103	92	96	92
query43	481	431	439	431
query44	1112	707	699	699
query45	263	252	255	252
query46	1122	699	685	685
query47	1936	1839	1853	1839
query48	448	346	360	346
query49	1234	344	343	343
query50	764	371	366	366
query51	6743	6630	6662	6630
query52	116	90	90	90
query53	352	280	276	276
query54	332	245	259	245
query55	88	84	82	82
query56	257	229	229	229
query57	1220	1145	1130	1130
query58	248	209	210	209
query59	2722	2643	2663	2643
query60	267	246	249	246
query61	113	113	114	113
query62	665	480	474	474
query63	313	275	280	275
query64	6920	4072	4104	4072
query65	3134	3041	3041	3041
query66	1417	380	372	372
query67	15345	15098	14756	14756
query68	6032	519	528	519
query69	598	400	387	387
query70	1240	1194	1172	1172
query71	472	274	273	273
query72	6391	2841	2516	2516
query73	722	315	316	315
query74	6751	6385	6448	6385
query75	4089	2847	2763	2763
query76	4341	849	881	849
query77	595	255	272	255
query78	10913	10113	10283	10113
query79	8960	509	518	509
query80	2133	396	369	369
query81	548	212	225	212
query82	1443	201	207	201
query83	298	146	141	141
query84	289	78	81	78
query85	1689	333	315	315
query86	493	306	312	306
query87	3670	3567	3499	3499
query88	5052	2297	2272	2272
query89	515	369	368	368
query90	1948	175	173	173
query91	172	136	138	136
query92	65	46	46	46
query93	7027	498	477	477
query94	1232	174	177	174
query95	434	331	324	324
query96	608	269	273	269
query97	3096	2900	2843	2843
query98	230	206	221	206
query99	1177	918	917	917
Total cold run time: 315575 ms
Total hot run time: 186260 ms

@doris-robot
Copy link

Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Load test result on commit d8079226bddfa2fc4b39a211e12d24767e58dd54 with default session variables
Stream load json:         19 seconds loaded 2358488459 Bytes, about 118 MB/s
Stream load orc:          58 seconds loaded 1101869774 Bytes, about 18 MB/s
Stream load parquet:      32 seconds loaded 861443392 Bytes, about 25 MB/s
Insert into select:       22.4 seconds inserted 10000000 Rows, about 446K ops/s

Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@dataroaring dataroaring merged commit 55691ad into apache:master Mar 25, 2024
28 of 32 checks passed
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.

4 participants