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

[test](regression) add fault injection cases for LoadStream #29101

Merged
merged 1 commit into from
Dec 28, 2023
Merged

[test](regression) add fault injection cases for LoadStream #29101

merged 1 commit into from
Dec 28, 2023

Conversation

freemandealer
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

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

github-actions[bot]
github-actions bot reviewed Dec 26, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

be/src/runtime/load_stream.cpp
Comment on lines +45 to +46
#define UNKNOWN_ID_FOR_TEST 0x7c00

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: replace macro with enum [modernize-macro-to-enum]

Suggested change
#define UNKNOWN_ID_FOR_TEST 0x7c00
enum {
UNKNOWN_ID_FOR_TEST = 0x7c00};

be/src/runtime/load_stream.cpp
#include "util/runtime_profile.h"
#include "util/thrift_util.h"
#include "util/uid_util.h"

#define UNKNOWN_ID_FOR_TEST 0x7c00
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: macro 'UNKNOWN_ID_FOR_TEST' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define UNKNOWN_ID_FOR_TEST 0x7c00
        ^

dataroaring
dataroaring approved these changes Dec 26, 2023
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

@dataroaring
Copy link
Contributor

run buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 26, 2023
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.57% (8557/23399)
Line Coverage: 28.64% (69628/243080)
Region Coverage: 27.64% (36002/130250)
Branch Coverage: 24.36% (18396/75506)
Coverage Report: http://coverage.selectdb-in.cc/coverage/84e1c787283caabb292663d399d893707d80b270_84e1c787283caabb292663d399d893707d80b270/report/index.html

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Tpch sf100 test result on commit 84e1c787283caabb292663d399d893707d80b270, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4776	4455	4469	4455
q2	365	148	159	148
q3	1473	1267	1198	1198
q4	1146	955	912	912
q5	3164	3172	3164	3164
q6	259	137	135	135
q7	1037	488	482	482
q8	2249	2306	2252	2252
q9	6690	6681	6695	6681
q10	3210	3278	3287	3278
q11	340	212	220	212
q12	361	212	212	212
q13	4515	3811	3795	3795
q14	245	216	209	209
q15	577	517	539	517
q16	433	380	388	380
q17	1051	810	655	655
q18	7056	6822	6863	6822
q19	1577	1594	1596	1594
q20	583	291	317	291
q21	3244	2761	2773	2761
q22	372	315	307	307
Total cold run time: 44723 ms
Total hot run time: 40460 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4405	4391	4403	4391
q2	271	166	174	166
q3	3508	3497	3497	3497
q4	2429	2416	2405	2405
q5	5717	5734	5704	5704
q6	251	125	127	125
q7	2370	1815	1831	1815
q8	3621	3611	3606	3606
q9	8998	9017	8949	8949
q10	3927	4030	3999	3999
q11	493	356	365	356
q12	775	606	605	605
q13	4269	3541	3552	3541
q14	286	267	248	248
q15	574	528	525	525
q16	510	466	455	455
q17	1963	1956	1954	1954
q18	8686	8324	8285	8285
q19	1828	1828	1828	1828
q20	2253	1952	1939	1939
q21	6628	6236	6241	6236
q22	543	474	463	463
Total cold run time: 64305 ms
Total hot run time: 61092 ms

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.86 seconds
stream load tsv: 567 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 20.8 seconds inserted 10000000 Rows, about 480K ops/s
storage size: 17183725773 Bytes

@freemandealer freemandealer changed the title [WIP][test](regression) add fault injection cases for LoadStream [test](regression) add fault injection cases for LoadStream Dec 28, 2023
@freemandealer
Copy link
Contributor Author

run external

dataroaring
dataroaring approved these changes Dec 28, 2023
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

sollhui
sollhui approved these changes Dec 28, 2023
View reviewed changes
Copy link
Contributor

@sollhui sollhui 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 b31494b into apache:master Dec 28, 2023
28 of 31 checks passed
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Jan 12, 2024
@freemandealer @HappenLee
[test](regression) add fault injection cases for LoadStream (apache#2…
188dfca 
…9101)

Signed-off-by: freemandealer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions GitHub Actions left review comments

@sollhui sollhui sollhui approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@freemandealer @dataroaring @doris-robot @sollhui