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](merge-on-write) remove some CHECKs in Tablet::revise_tablet_meta #31268

Conversation

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

Copy link
Contributor

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

1 similar comment
Copy link
Contributor

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

Copy link
Contributor

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

@zhannngchen zhannngchen force-pushed the fix-clone-core-on-delete-bitmap-calculation branch from b7267d2 to ca1851d Compare March 6, 2024 11:35
Copy link
Contributor

github-actions bot commented Mar 6, 2024

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

@zhannngchen zhannngchen force-pushed the fix-clone-core-on-delete-bitmap-calculation branch from ca1851d to f60697e Compare March 15, 2024 14:23
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

DBUG_EXECUTE_IF("Tablet.update_delete_bitmap_without_lock.random_failed", {
if (rand() % 100 < (100 * dp->param("percent", 0.1))) {
LOG_WARNING("Tablet.update_delete_bitmap_without_lock.random_failed");
Status BaseTablet::update_delete_bitmap_without_lock(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'update_delete_bitmap_without_lock' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status BaseTablet::update_delete_bitmap_without_lock(
static Status BaseTablet::update_delete_bitmap_without_lock(

@zhannngchen zhannngchen marked this pull request as ready for review April 30, 2024 04:29
@zhannngchen
Copy link
Contributor Author

run buildall

Copy link
Contributor

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

@doris-robot
Copy link

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

query1	912	357	345	345
query2	6462	2375	2344	2344
query3	6649	204	201	201
query4	23376	21769	21886	21769
query5	3727	437	417	417
query6	266	177	172	172
query7	4586	311	299	299
query8	248	206	196	196
query9	8450	2347	2333	2333
query10	405	244	271	244
query11	11821	11187	11464	11187
query12	127	91	91	91
query13	1701	378	371	371
query14	9841	7511	8137	7511
query15	260	177	176	176
query16	8239	268	274	268
query17	1911	563	556	556
query18	2109	293	266	266
query19	336	150	152	150
query20	93	84	86	84
query21	194	131	122	122
query22	5094	4802	4766	4766
query23	33883	33171	33293	33171
query24	10592	2945	2894	2894
query25	623	376	381	376
query26	1117	150	151	150
query27	2339	320	315	315
query28	7013	2010	2011	2010
query29	872	635	612	612
query30	251	209	149	149
query31	970	744	730	730
query32	92	50	54	50
query33	748	234	236	234
query34	1056	477	467	467
query35	808	669	671	669
query36	1055	918	899	899
query37	128	64	63	63
query38	3138	3023	3060	3023
query39	1570	1543	1527	1527
query40	195	125	124	124
query41	39	37	40	37
query42	100	91	93	91
query43	563	556	537	537
query44	1215	732	743	732
query45	260	257	258	257
query46	1081	739	713	713
query47	1964	1876	1865	1865
query48	370	296	294	294
query49	853	393	377	377
query50	762	384	375	375
query51	6661	6610	6621	6610
query52	94	91	87	87
query53	348	278	284	278
query54	304	237	249	237
query55	81	75	69	69
query56	234	216	215	215
query57	1205	1164	1154	1154
query58	222	198	196	196
query59	3440	3260	3208	3208
query60	243	228	230	228
query61	86	85	85	85
query62	643	493	466	466
query63	310	279	276	276
query64	8631	7207	7248	7207
query65	3096	3073	3047	3047
query66	824	328	337	328
query67	15601	14973	15183	14973
query68	9639	559	540	540
query69	584	302	311	302
query70	1346	1094	1058	1058
query71	533	269	270	269
query72	8629	2533	2370	2370
query73	1582	322	331	322
query74	6584	6128	6160	6128
query75	5263	2659	2622	2622
query76	6041	1016	1005	1005
query77	800	270	261	261
query78	11027	10224	10288	10224
query79	12159	511	507	507
query80	1859	420	420	420
query81	494	224	222	222
query82	271	88	93	88
query83	199	160	161	160
query84	264	84	84	84
query85	1060	274	258	258
query86	355	308	304	304
query87	3384	3141	3126	3126
query88	5262	2401	2382	2382
query89	514	386	378	378
query90	2282	178	180	178
query91	126	98	98	98
query92	57	47	46	46
query93	7727	507	507	507
query94	1641	178	176	176
query95	385	301	297	297
query96	609	270	259	259
query97	3131	2959	2968	2959
query98	226	221	217	217
query99	1209	905	896	896
Total cold run time: 307729 ms
Total hot run time: 183936 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.57% (8934/25115)
Line Coverage: 27.20% (73703/270967)
Region Coverage: 26.39% (38063/144245)
Branch Coverage: 23.17% (19404/83738)
Coverage Report: http://coverage.selectdb-in.cc/coverage/c234bb032bef6804b90c4d16a93a8aff838dff96_c234bb032bef6804b90c4d16a93a8aff838dff96/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 May 7, 2024
Copy link
Contributor

github-actions bot commented May 7, 2024

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

Copy link
Contributor

github-actions bot commented May 7, 2024

PR approved by anyone and no changes requested.

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 f1b6bc4 into apache:master May 8, 2024
25 of 28 checks passed
zhannngchen added a commit to zhannngchen/incubator-doris that referenced this pull request May 11, 2024
zhannngchen added a commit to zhannngchen/incubator-doris that referenced this pull request Jun 1, 2024
yiguolei pushed a commit that referenced this pull request Jun 1, 2024
…_meta (#31268) (#34702)

## Proposed changes

Issue Number: close #xxx

cherry-pick #31268 

## Further comments

If this is a relatively large or complex change, kick off the discussion
at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why
you chose the solution you did and what alternatives you considered,
etc...
zhannngchen added a commit to zhannngchen/incubator-doris that referenced this pull request Jun 13, 2024
…_meta (apache#31268) (apache#34702)

## Proposed changes

Issue Number: close #xxx

cherry-pick apache#31268 

## Further comments

If this is a relatively large or complex change, kick off the discussion
at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why
you chose the solution you did and what alternatives you considered,
etc...
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.12-merged dev/2.1.4-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants