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](schema-change) Remove incorrect assertion for tablet state after committing sc job #48219

Merged
merged 1 commit into from
Feb 27, 2025
Merged

[fix](schema-change) Remove incorrect assertion for tablet state after committing sc job #48219

merged 1 commit into from
Feb 27, 2025

Conversation

TangSiyang2001
Copy link
Collaborator

@TangSiyang2001 TangSiyang2001 commented Feb 24, 2025 •
edited
Loading

What problem does this PR solve?

Problem Summary:

After modification #45821, tablet state cloud have been updated before schema change job updating BE local tablet state.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@TangSiyang2001
Copy link
Collaborator Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17609	5343	5135	5135
q2	2055	284	163	163
q3	10423	1255	758	758
q4	10207	1012	541	541
q5	7537	2465	2284	2284
q6	189	177	135	135
q7	913	772	590	590
q8	9324	1348	1183	1183
q9	4884	4645	4580	4580
q10	6916	2304	1886	1886
q11	477	271	265	265
q12	370	361	230	230
q13	17809	3680	3064	3064
q14	236	223	207	207
q15	511	469	468	468
q16	622	617	571	571
q17	573	871	352	352
q18	6784	6276	6242	6242
q19	1688	964	555	555
q20	312	326	193	193
q21	2798	2228	1932	1932
q22	363	329	301	301
Total cold run time: 102600 ms
Total hot run time: 31635 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5202	5209	5225	5209
q2	242	356	230	230
q3	2233	2718	2330	2330
q4	1479	1863	1440	1440
q5	4245	4126	4176	4126
q6	211	167	126	126
q7	1878	1830	1738	1738
q8	2613	2678	2600	2600
q9	7253	7197	7236	7197
q10	3084	3219	2814	2814
q11	582	512	493	493
q12	725	794	648	648
q13	3403	3988	3286	3286
q14	288	294	272	272
q15	516	460	460	460
q16	645	681	640	640
q17	1158	1604	1352	1352
q18	7628	7295	7314	7295
q19	855	882	917	882
q20	2026	2037	1869	1869
q21	5453	5027	4882	4882
q22	612	611	563	563
Total cold run time: 52331 ms
Total hot run time: 50452 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 190732 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 0414e79945113cf1550b5a03e9e26fa85a81f687, data reload: false

query1	1310	934	955	934
query2	6252	1875	1844	1844
query3	11138	4482	4600	4482
query4	56447	24921	23343	23343
query5	5266	541	505	505
query6	346	191	198	191
query7	4996	503	287	287
query8	316	240	227	227
query9	5787	2574	2579	2574
query10	414	311	245	245
query11	15175	15260	15030	15030
query12	157	102	108	102
query13	1077	502	365	365
query14	10569	6929	6597	6597
query15	201	185	185	185
query16	6815	664	509	509
query17	1089	755	582	582
query18	918	392	323	323
query19	214	207	176	176
query20	128	129	119	119
query21	218	137	117	117
query22	4616	4653	4619	4619
query23	33854	33457	33535	33457
query24	5901	2448	2432	2432
query25	474	497	468	468
query26	684	290	159	159
query27	1810	501	338	338
query28	2822	2454	2420	2420
query29	529	546	411	411
query30	207	190	157	157
query31	915	867	814	814
query32	72	65	58	58
query33	458	353	292	292
query34	749	877	497	497
query35	816	835	732	732
query36	1007	983	901	901
query37	116	98	78	78
query38	4313	4310	4267	4267
query39	1476	1586	1427	1427
query40	206	116	105	105
query41	53	50	50	50
query42	124	104	108	104
query43	514	528	492	492
query44	1278	783	785	783
query45	186	170	163	163
query46	895	1061	641	641
query47	1881	1889	1789	1789
query48	397	430	303	303
query49	687	496	426	426
query50	699	769	413	413
query51	4257	4286	4276	4276
query52	100	104	99	99
query53	225	252	183	183
query54	470	476	411	411
query55	85	77	80	77
query56	265	282	267	267
query57	1168	1203	1145	1145
query58	271	238	232	232
query59	2817	2932	2786	2786
query60	280	292	265	265
query61	121	120	115	115
query62	772	738	660	660
query63	230	197	186	186
query64	1440	984	651	651
query65	3192	3113	3125	3113
query66	680	394	294	294
query67	16149	15728	15400	15400
query68	5503	760	517	517
query69	534	285	268	268
query70	1178	1134	1096	1096
query71	431	303	258	258
query72	5824	3662	3699	3662
query73	1099	741	349	349
query74	9132	8932	8894	8894
query75	3254	3160	2654	2654
query76	3860	1173	740	740
query77	530	371	280	280
query78	10039	10348	9294	9294
query79	1431	868	599	599
query80	599	543	423	423
query81	488	281	233	233
query82	212	130	94	94
query83	164	174	152	152
query84	290	99	72	72
query85	774	338	296	296
query86	339	300	285	285
query87	4391	4577	4442	4442
query88	2813	2227	2205	2205
query89	390	326	279	279
query90	1778	188	188	188
query91	130	201	107	107
query92	68	58	58	58
query93	1807	999	571	571
query94	629	397	287	287
query95	340	259	256	256
query96	500	539	266	266
query97	2740	2858	2748	2748
query98	224	200	204	200
query99	1295	1407	1266	1266
Total cold run time: 291880 ms
Total hot run time: 190732 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 44.50% (11750/26407)
Line Coverage: 33.99% (98207/288954)
Region Coverage: 33.07% (50303/152113)
Branch Coverage: 28.71% (25283/88054)
Coverage Report: http://coverage.selectdb-in.cc/coverage/0414e79945113cf1550b5a03e9e26fa85a81f687_0414e79945113cf1550b5a03e9e26fa85a81f687/report/index.html

@doris-robot
Copy link

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

query1	0.03	0.05	0.03
query2	0.07	0.03	0.03
query3	0.23	0.07	0.07
query4	1.62	0.10	0.11
query5	0.43	0.40	0.42
query6	1.16	0.66	0.66
query7	0.02	0.01	0.01
query8	0.04	0.03	0.04
query9	0.60	0.51	0.53
query10	0.58	0.58	0.57
query11	0.15	0.10	0.10
query12	0.14	0.10	0.11
query13	0.61	0.61	0.60
query14	2.67	2.69	2.72
query15	0.91	0.85	0.84
query16	0.38	0.37	0.38
query17	1.02	1.10	1.04
query18	0.21	0.20	0.19
query19	1.92	1.73	1.95
query20	0.01	0.01	0.01
query21	15.42	0.92	0.53
query22	0.76	1.30	0.70
query23	14.76	1.38	0.64
query24	6.96	1.61	0.66
query25	0.53	0.18	0.14
query26	0.57	0.15	0.14
query27	0.05	0.05	0.05
query28	9.71	0.83	0.43
query29	12.55	3.86	3.25
query30	0.24	0.09	0.06
query31	2.82	0.57	0.37
query32	3.22	0.54	0.45
query33	2.95	3.03	3.02
query34	15.78	5.15	4.51
query35	4.56	4.57	4.60
query36	0.67	0.50	0.48
query37	0.10	0.06	0.06
query38	0.05	0.04	0.03
query39	0.03	0.02	0.02
query40	0.18	0.12	0.12
query41	0.08	0.03	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 104.87 s
Total hot run time: 30.38 s

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

luwei16
luwei16 reviewed Feb 26, 2025
View reviewed changes
Copy link
Contributor

@luwei16 luwei16 left a comment

Choose a reason for hiding this comment

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

should DCHECK be moved before commit_tablet_job

@TangSiyang2001
Copy link
Collaborator Author

TangSiyang2001 commented Feb 27, 2025 •
edited
Loading

should DCHECK be moved before commit_tablet_job

after discussion, we agree on it's not necessary.

@dataroaring dataroaring merged commit 8e2b4a1 into apache:master Feb 27, 2025
27 of 30 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 27, 2025
@TangSiyang2001
[fix](schema-change) Remove incorrect assertion for tablet state afte…
b545dd3 
…r committing sc job (#48219)

After modification #45821, tablet state cloud have been updated before
schema change job updating BE local tablet state.
@github-actions github-actions bot mentioned this pull request Feb 27, 2025
Merged
seawinde pushed a commit to seawinde/doris that referenced this pull request Feb 28, 2025
@TangSiyang2001 @seawinde
[fix](schema-change) Remove incorrect assertion for tablet state afte…
0764abb 
…r committing sc job (apache#48219)

After modification apache#45821, tablet state cloud have been updated before
schema change job updating BE local tablet state.
mymeiyi pushed a commit to mymeiyi/doris that referenced this pull request Mar 4, 2025
@TangSiyang2001 @mymeiyi
[fix](schema-change) Remove incorrect assertion for tablet state afte…
21fd930 
…r committing sc job (apache#48219)

After modification apache#45821, tablet state cloud have been updated before
schema change job updating BE local tablet state.
dataroaring pushed a commit that referenced this pull request Mar 13, 2025
@github-actions @TangSiyang2001
branch-3.0: [fix](schema-change) Remove incorrect assertion for table…
b24ac3b 
…t state after committing sc job #48219 (#48439)

Cherry-picked from #48219

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

@gavinchou gavinchou gavinchou approved these changes

@luwei16 luwei16 luwei16 approved these changes

@dataroaring dataroaring Awaiting requested review from dataroaring dataroaring is a code owner

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

Successfully merging this pull request may close these issues.
6 participants
@TangSiyang2001 @hello-stephen @doris-robot @gavinchou @luwei16 @dataroaring