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.

Sign up for GitHub

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

Jump to bottom

[fix](SSL) Correctly close SSL connections #38587

Merged
merged 1 commit into from
Jul 31, 2024
Merged

[fix](SSL) Correctly close SSL connections #38587

merged 1 commit into from
Jul 31, 2024

Conversation

TangSiyang2001
Copy link
Collaborator

@TangSiyang2001 TangSiyang2001 commented Jul 31, 2024 •
edited
Loading

Proposed changes

Issue Number: close #38590

If SSL connection closed, a specified packet will sent to indicate the closing of connection. The SSL engine will be shut down and output an empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by breaking the reading flow and do the cleanup stuff initiatively.

@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.

@TangSiyang2001 TangSiyang2001 changed the title [fix](SSL) Correctly close SSL connection [fix](SSL) Correctly close SSL connections Jul 31, 2024
@TangSiyang2001
Copy link
Collaborator Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@TangSiyang2001
Copy link
Collaborator Author

run buildall

@TangSiyang2001
Copy link
Collaborator Author

run feut

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17613	4193	4051	4051
q2	2036	203	198	198
q3	10454	1328	1371	1328
q4	10166	824	1002	824
q5	7675	2976	2986	2976
q6	218	137	151	137
q7	1031	616	614	614
q8	9443	1930	1965	1930
q9	8456	6999	6627	6627
q10	8755	4093	3856	3856
q11	432	250	260	250
q12	407	223	220	220
q13	17776	2935	2916	2916
q14	272	234	240	234
q15	529	486	496	486
q16	522	401	390	390
q17	972	893	904	893
q18	7990	7309	7259	7259
q19	1439	1242	1219	1219
q20	563	352	336	336
q21	5254	4825	4737	4737
q22	347	288	282	282
Total cold run time: 112350 ms
Total hot run time: 41763 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4117	4011	4017	4011
q2	330	226	216	216
q3	2994	3015	3198	3015
q4	2009	2041	1953	1953
q5	5643	5474	5436	5436
q6	219	131	134	131
q7	2156	1857	1795	1795
q8	3345	3430	3397	3397
q9	8655	8647	8813	8647
q10	3913	4074	3910	3910
q11	554	458	467	458
q12	812	622	614	614
q13	12980	3140	3089	3089
q14	332	278	279	278
q15	539	481	472	472
q16	459	411	422	411
q17	1814	1761	1734	1734
q18	8245	7688	7683	7683
q19	1711	1713	1725	1713
q20	2084	1828	1848	1828
q21	5712	5486	5240	5240
q22	520	467	460	460
Total cold run time: 69143 ms
Total hot run time: 56491 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 169204 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 6796d25229f8a591b636f15df1bb031236cca872, data reload: false

query1	921	371	356	356
query2	6482	1742	1666	1666
query3	6657	222	228	222
query4	20397	17373	17266	17266
query5	3615	521	531	521
query6	290	174	168	168
query7	4605	319	299	299
query8	258	205	196	196
query9	8512	2383	2381	2381
query10	424	279	287	279
query11	10471	9994	9905	9905
query12	121	91	87	87
query13	1647	384	379	379
query14	8787	7965	7071	7071
query15	204	160	164	160
query16	6937	489	398	398
query17	952	572	553	553
query18	1924	298	293	293
query19	200	153	149	149
query20	88	89	85	85
query21	206	102	103	102
query22	4266	4037	3960	3960
query23	33644	33680	33399	33399
query24	10241	3134	3053	3053
query25	704	444	452	444
query26	1675	159	156	156
query27	2903	291	290	290
query28	7513	2026	2019	2019
query29	1368	431	431	431
query30	238	152	156	152
query31	965	782	755	755
query32	99	55	54	54
query33	674	317	322	317
query34	916	496	518	496
query35	851	743	755	743
query36	1033	886	860	860
query37	272	80	79	79
query38	2971	2837	2844	2837
query39	910	794	806	794
query40	286	108	110	108
query41	48	45	46	45
query42	121	103	117	103
query43	474	413	429	413
query44	1193	735	730	730
query45	209	179	178	178
query46	1076	820	789	789
query47	1818	1689	1730	1689
query48	381	293	293	293
query49	982	419	407	407
query50	902	434	435	434
query51	6715	6663	6645	6645
query52	102	93	89	89
query53	249	190	179	179
query54	621	445	456	445
query55	81	74	75	74
query56	277	261	255	255
query57	1143	1050	1032	1032
query58	274	278	275	275
query59	2679	2374	2550	2374
query60	304	276	272	272
query61	144	94	93	93
query62	867	658	673	658
query63	209	182	182	182
query64	5708	1897	1884	1884
query65	3178	3062	3094	3062
query66	1308	333	378	333
query67	15311	14909	14819	14819
query68	4488	557	571	557
query69	591	368	294	294
query70	1117	1066	1117	1066
query71	504	276	274	274
query72	7862	2714	2472	2472
query73	756	329	329	329
query74	6023	5621	5593	5593
query75	3990	2717	2702	2702
query76	3062	1295	1398	1295
query77	686	311	302	302
query78	9458	8864	8826	8826
query79	2875	540	530	530
query80	2204	542	494	494
query81	565	229	224	224
query82	897	186	126	126
query83	304	170	176	170
query84	271	77	81	77
query85	1305	329	307	307
query86	454	321	303	303
query87	3249	3026	3139	3026
query88	4004	2441	2407	2407
query89	387	291	304	291
query90	1981	188	195	188
query91	125	140	99	99
query92	59	50	50	50
query93	2327	616	628	616
query94	986	282	315	282
query95	383	275	267	267
query96	606	278	278	278
query97	3244	3080	3044	3044
query98	218	197	191	191
query99	1680	1293	1289	1289
Total cold run time: 268262 ms
Total hot run time: 169204 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 29.96 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 6796d25229f8a591b636f15df1bb031236cca872, data reload: false

query1	0.05	0.04	0.03
query2	0.08	0.04	0.05
query3	0.22	0.05	0.05
query4	1.70	0.06	0.06
query5	0.49	0.49	0.49
query6	1.14	0.72	0.72
query7	0.02	0.02	0.02
query8	0.06	0.05	0.04
query9	0.57	0.51	0.51
query10	0.57	0.56	0.55
query11	0.16	0.11	0.12
query12	0.16	0.12	0.12
query13	0.61	0.60	0.59
query14	0.76	0.81	0.79
query15	0.90	0.86	0.86
query16	0.34	0.35	0.36
query17	1.00	1.00	0.97
query18	0.21	0.21	0.22
query19	1.87	1.74	1.77
query20	0.01	0.01	0.01
query21	15.39	0.76	0.64
query22	3.79	7.60	1.35
query23	18.13	1.38	1.31
query24	2.27	0.23	0.21
query25	0.17	0.08	0.08
query26	0.33	0.22	0.22
query27	0.46	0.23	0.23
query28	13.16	1.01	0.97
query29	12.57	3.23	3.23
query30	0.25	0.06	0.05
query31	2.86	0.40	0.40
query32	3.25	0.48	0.49
query33	2.91	2.93	2.91
query34	15.42	4.27	4.24
query35	4.30	4.31	4.29
query36	0.68	0.48	0.49
query37	0.19	0.16	0.17
query38	0.17	0.15	0.15
query39	0.04	0.03	0.03
query40	0.16	0.13	0.13
query41	0.10	0.06	0.06
query42	0.06	0.05	0.04
query43	0.04	0.04	0.04
Total cold run time: 107.62 s
Total hot run time: 29.96 s

@TangSiyang2001 TangSiyang2001 marked this pull request as ready for review July 31, 2024 13:02
dataroaring
dataroaring approved these changes Jul 31, 2024
View reviewed changes
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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 31, 2024
@dataroaring dataroaring merged commit a03a1c2 into apache:master Jul 31, 2024
30 of 33 checks passed
TangSiyang2001 added a commit to TangSiyang2001/doris that referenced this pull request Aug 1, 2024
@TangSiyang2001
[fix](SSL) Correctly close SSL connections (apache#38587)
42e0127 
## Proposed changes

Issue Number: close apache#38590 

If SSL connection closed, a specified packet will sent to indicate the
closing of connection. The SSL engine will be shut down and output an
empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by
breaking the reading flow and do the cleanup stuff initiatively.
dataroaring pushed a commit that referenced this pull request Aug 1, 2024
@TangSiyang2001
[cherry-pick](SSL) Fix ssl connection close 2.1 (#38587) (#38677)
cafcf7a 
## Proposed changes

Issue Number: close #38590 

If SSL connection closed, a specified packet will sent to indicate the
closing of connection. The SSL engine will be shut down and output an
empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by
breaking the reading flow and do the cleanup stuff initiatively.
feiniaofeiafei pushed a commit to feiniaofeiafei/doris that referenced this pull request Aug 9, 2024
@TangSiyang2001 @feiniaofeiafei
[fix](SSL) Correctly close SSL connections (apache#38587)
13a4d88 
## Proposed changes

Issue Number: close apache#38590 

If SSL connection closed, a specified packet will sent to indicate the
closing of connection. The SSL engine will be shut down and output an
empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by
breaking the reading flow and do the cleanup stuff initiatively.
dataroaring pushed a commit that referenced this pull request Aug 11, 2024
@TangSiyang2001 @dataroaring
[fix](SSL) Correctly close SSL connections (#38587)
a7f9cd9 
## Proposed changes

Issue Number: close #38590 

If SSL connection closed, a specified packet will sent to indicate the
closing of connection. The SSL engine will be shut down and output an
empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by
breaking the reading flow and do the cleanup stuff initiatively.
TangSiyang2001 added a commit to TangSiyang2001/doris that referenced this pull request Aug 14, 2024
@TangSiyang2001
Revert "[fix](SSL) Correctly close SSL connections (apache#38587)"
35f5e90 
This reverts commit a03a1c2.
dataroaring pushed a commit that referenced this pull request Aug 16, 2024
@TangSiyang2001 @dataroaring
[fix](SSL) Correctly close SSL connections (#38587)
fb28727 
## Proposed changes

Issue Number: close #38590 

If SSL connection closed, a specified packet will sent to indicate the
closing of connection. The SSL engine will be shut down and output an
empty unwrapped result.

Therefore, handle this case correctly to avoid buffer overflow by
breaking the reading flow and do the cleanup stuff initiatively.
@gavinchou gavinchou mentioned this pull request Oct 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yukang-Lian Yukang-Lian Yukang-Lian approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.6-merged dev/3.0.2-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Unexcepted exceptions with mysql SSL sessions
4 participants
@TangSiyang2001 @doris-robot @Yukang-Lian @dataroaring