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](hdfs) Fix hdfsExists that return staled root cause #27991

Merged

Conversation

w41ter
Copy link
Contributor

@w41ter w41ter commented Dec 5, 2023

Proposed changes

Issue Number: close #xxx

The HDFS native client won't clear the last exception as expected so hdfsGetLastExceptionRootCause might return a staled root cause. This PR saves the last root cause here and verifies after hdfsExists returns a non-zero code.

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

Copy link
Contributor

github-actions bot commented Dec 5, 2023

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

dataroaring
dataroaring previously approved these changes Dec 5, 2023
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 bot added the approved Indicates a PR has been approved by one committer. label Dec 5, 2023
Copy link
Contributor

github-actions bot commented Dec 5, 2023

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

Copy link
Contributor

github-actions bot commented Dec 5, 2023

PR approved by anyone and no changes requested.

@w41ter
Copy link
Contributor Author

w41ter commented Dec 5, 2023

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Dec 5, 2023
Copy link
Contributor

github-actions bot commented Dec 5, 2023

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

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit a30233c0af46375b4c6748d952f62cde094c1404, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4656	4406	4487	4406
q2	362	153	186	153
q3	1466	1301	1200	1200
q4	1104	934	886	886
q5	3143	3207	3204	3204
q6	251	125	129	125
q7	989	489	488	488
q8	2231	2228	2163	2163
q9	6695	6736	6684	6684
q10	3204	3241	3272	3241
q11	322	211	206	206
q12	351	213	213	213
q13	5188	3812	3769	3769
q14	247	211	213	211
q15	573	523	520	520
q16	433	382	381	381
q17	1013	605	553	553
q18	7951	7572	8896	7572
q19	1515	1387	1431	1387
q20	551	503	306	306
q21	3075	2716	2719	2716
q22	358	287	294	287
Total cold run time: 45678 ms
Total hot run time: 40671 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4384	4391	4363	4363
q2	269	161	177	161
q3	3523	3524	3513	3513
q4	2387	2382	2362	2362
q5	5749	5737	5726	5726
q6	236	122	122	122
q7	2383	1850	1835	1835
q8	3517	3543	3537	3537
q9	9116	9041	9054	9041
q10	3914	4012	3997	3997
q11	520	367	383	367
q12	769	610	589	589
q13	4346	3574	3529	3529
q14	272	243	247	243
q15	566	515	519	515
q16	486	439	452	439
q17	1866	1864	1831	1831
q18	8588	8243	9067	8243
q19	1746	1754	1756	1754
q20	2261	1942	1960	1942
q21	6546	6153	6157	6153
q22	498	422	428	422
Total cold run time: 63942 ms
Total hot run time: 60684 ms

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.23 seconds
stream load tsv: 567 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17166941866 Bytes

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

Copy link
Contributor

github-actions bot commented Dec 6, 2023

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 Dec 6, 2023
@morningman morningman merged commit 7340440 into apache:master Dec 6, 2023
27 of 29 checks passed
@w41ter w41ter deleted the fix/hdfs_native_client_staled_root_cause branch December 6, 2023 02:15
XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
The HDFS native client won't clear the last exception as expected so `hdfsGetLastExceptionRootCause` might return a staled root cause. This PR saves the last root cause here and verifies after hdfsExists returns a non-zero code.
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.

5 participants