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](jni) avoid coredump if failed to get jni env #32950

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

morningman
Copy link
Contributor

@morningman morningman commented Mar 28, 2024

Proposed changes

This PR #32217 find a problem that may failed to get jni env.
And it did a work around to avoid BE crash.

This PR followup this issue, to avoid BE crash when doing close() of JniConnector
if failed to get jni env.

The close() method will return error when:

  1. Failed to get jni env
  2. Failed to release jni resource.

This PR will ignore the first error, and still log fatal for second error

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 morningman marked this pull request as ready for review March 28, 2024 04:41
Copy link
Contributor

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

@morningman
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.63% (8835/24797)
Line Coverage: 27.34% (72402/264813)
Region Coverage: 26.57% (37557/141350)
Branch Coverage: 23.38% (19155/81946)
Coverage Report: http://coverage.selectdb-in.cc/coverage/94ac8770e3b0e09a339710932ded9d6aac8442c3_94ac8770e3b0e09a339710932ded9d6aac8442c3/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17633	4186	4266	4186
q2	2114	166	151	151
q3	10584	1178	1238	1178
q4	10244	785	758	758
q5	7466	2867	2862	2862
q6	201	122	123	122
q7	1050	571	571	571
q8	9332	1993	1979	1979
q9	6970	6367	6303	6303
q10	8443	3466	3560	3466
q11	433	225	219	219
q12	414	200	198	198
q13	17797	2859	2862	2859
q14	240	214	203	203
q15	517	462	461	461
q16	472	366	370	366
q17	941	550	592	550
q18	6962	6410	6409	6409
q19	3252	1438	1392	1392
q20	541	255	246	246
q21	3641	2952	2888	2888
q22	337	287	299	287
Total cold run time: 109584 ms
Total hot run time: 37654 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4127	4088	4120	4088
q2	325	226	226	226
q3	2926	2829	2807	2807
q4	1824	1602	1532	1532
q5	5201	5213	5217	5213
q6	194	117	120	117
q7	2192	1858	1812	1812
q8	3155	3274	3316	3274
q9	8300	8356	8337	8337
q10	3750	3793	3784	3784
q11	547	443	446	443
q12	712	536	536	536
q13	16942	2823	2840	2823
q14	291	249	268	249
q15	496	463	451	451
q16	452	423	411	411
q17	1731	1473	1464	1464
q18	7373	7149	7000	7000
q19	1637	1525	1514	1514
q20	1901	1721	1696	1696
q21	4864	4566	4592	4566
q22	528	447	445	445
Total cold run time: 69468 ms
Total hot run time: 52788 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 182055 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 94ac8770e3b0e09a339710932ded9d6aac8442c3, data reload: false

query1	943	371	345	345
query2	6792	1902	1875	1875
query3	6721	209	210	209
query4	31841	21286	21229	21229
query5	4337	400	391	391
query6	278	182	181	181
query7	4621	288	282	282
query8	229	170	172	170
query9	9106	2322	2331	2322
query10	552	256	281	256
query11	17273	14207	14209	14207
query12	134	97	88	88
query13	1615	407	407	407
query14	10201	7854	7881	7854
query15	320	192	186	186
query16	8243	262	260	260
query17	2034	594	551	551
query18	2123	296	294	294
query19	368	166	164	164
query20	104	86	87	86
query21	206	128	130	128
query22	5013	4831	4817	4817
query23	33576	33008	33061	33008
query24	10700	2851	2869	2851
query25	616	391	391	391
query26	1166	153	155	153
query27	2630	356	351	351
query28	7163	1906	1875	1875
query29	903	629	630	629
query30	302	152	151	151
query31	971	719	746	719
query32	98	60	57	57
query33	776	261	254	254
query34	1061	482	493	482
query35	818	605	607	605
query36	1000	887	867	867
query37	119	60	63	60
query38	3581	3420	3462	3420
query39	1566	1469	1425	1425
query40	206	117	112	112
query41	48	45	46	45
query42	104	94	92	92
query43	505	458	462	458
query44	1213	721	739	721
query45	265	271	255	255
query46	1103	719	713	713
query47	1923	1890	1883	1883
query48	432	353	360	353
query49	1094	343	321	321
query50	757	380	368	368
query51	6882	6763	6788	6763
query52	95	94	90	90
query53	343	269	272	269
query54	293	233	239	233
query55	83	75	78	75
query56	241	221	225	221
query57	1239	1172	1150	1150
query58	226	200	202	200
query59	2697	2620	2560	2560
query60	257	245	251	245
query61	98	93	93	93
query62	649	421	417	417
query63	298	272	275	272
query64	5611	3971	4031	3971
query65	3080	3012	3033	3012
query66	882	379	368	368
query67	15359	14862	14762	14762
query68	6661	521	529	521
query69	624	389	389	389
query70	1227	1144	1157	1144
query71	498	262	265	262
query72	6481	2659	2458	2458
query73	709	316	322	316
query74	8180	6413	6404	6404
query75	3630	2210	2218	2210
query76	4467	967	924	924
query77	680	265	279	265
query78	10918	10194	10157	10157
query79	8051	528	532	528
query80	1678	407	366	366
query81	534	213	224	213
query82	1611	83	85	83
query83	230	152	150	150
query84	288	88	81	81
query85	1578	316	301	301
query86	480	325	299	299
query87	3755	3531	3511	3511
query88	5113	2318	2325	2318
query89	510	367	367	367
query90	1992	174	175	174
query91	170	135	138	135
query92	57	48	47	47
query93	6477	497	492	492
query94	1163	178	179	178
query95	425	328	324	324
query96	627	266	272	266
query97	2655	2519	2518	2518
query98	229	214	211	211
query99	1126	828	821	821
Total cold run time: 308907 ms
Total hot run time: 182055 ms

@doris-robot
Copy link

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

Load test result on commit 94ac8770e3b0e09a339710932ded9d6aac8442c3 with default session variables
Stream load json:         18 seconds loaded 2358488459 Bytes, about 124 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      31 seconds loaded 861443392 Bytes, about 26 MB/s
Insert into select:       13.6 seconds inserted 10000000 Rows, about 735K ops/s

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 29, 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.

@morningman morningman merged commit f19ba76 into apache:master Mar 29, 2024
28 of 32 checks passed
morningman added a commit to morningman/doris that referenced this pull request Mar 29, 2024
This PR apache#32217 find a problem that may failed to get jni env.
And it did a work around to avoid BE crash.

This PR followup this issue, to avoid BE crash when doing `close()` of JniConnector
if failed to get jni env.

The `close()` method will return error when:
1. Failed to get jni env
2. Failed to release jni resource.

This PR will ignore the first error, and still log fatal for second error
yiguolei pushed a commit that referenced this pull request Apr 1, 2024
This PR #32217 find a problem that may failed to get jni env.
And it did a work around to avoid BE crash.

This PR followup this issue, to avoid BE crash when doing `close()` of JniConnector
if failed to get jni env.

The `close()` method will return error when:
1. Failed to get jni env
2. Failed to release jni resource.

This PR will ignore the first error, and still log fatal for second error
morningman added a commit to morningman/doris that referenced this pull request Apr 7, 2024
This PR apache#32217 find a problem that may failed to get jni env.
And it did a work around to avoid BE crash.

This PR followup this issue, to avoid BE crash when doing `close()` of JniConnector
if failed to get jni env.

The `close()` method will return error when:
1. Failed to get jni env
2. Failed to release jni resource.

This PR will ignore the first error, and still log fatal for second error
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.0.8-merged dev/2.1.2-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants