Skip to content

Conversation

@Hastyshell
Copy link
Collaborator

What problem does this PR solve?

Problem Summary:

  1. New tablet cloud be nullptr when get tablet action failed, take it into account.
  2. Fix grammer error of the method signature.

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?

dataroaring
dataroaring previously approved these changes Jul 28, 2025
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 Jul 28, 2025
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@dataroaring
Copy link
Contributor

run buildall

void CloudSchemaChangeJob::clean_up_on_failed() {
void CloudSchemaChangeJob::clean_up_on_failure() {
if (_new_tablet == nullptr) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

log here?

Copy link
Collaborator Author

@Hastyshell Hastyshell Jul 28, 2025

Choose a reason for hiding this comment

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

I think log here is pointless.
If get tablet failed, we could find details through log msg like this one:

W20250727 00:45:38.585387  5797 cloud_tablet_mgr.cpp:206] failed to tablet 1753547556254: [INVALID_ARGUMENT]failed to get tablet meta: empty instance_id
W20250727 00:45:38.599696  5797 status.h:426] meet error status: [INTERNAL_ERROR]failed to get tablet 1753547556254
        0#  doris::CloudTabletMgr::get_tablet(long, bool, bool, doris::SyncRowsetStats*, bool) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/common/status.h:378
        1#  doris::CloudSchemaChangeJob::process_alter_tablet(doris::TAlterTabletReqV2 const&) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/util/expected.hpp:1986
        2#  doris::alter_cloud_tablet_callback(doris::CloudStorageEngine&, doris::TAgentTaskRequest const&) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/common/status.h:505
        3#  std::_Function_handler<void (), doris::TaskWorkerPool::submit_task(doris::TAgentTaskRequest const&)::$_0::operator()<doris::TAgentTaskRequest const&>(doris::TAgentTaskRequest const&) const::{lambda()#1}>::_M_invoke(std::_Any_data const&) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/agent/task_worker_pool.cpp:546
        4#  doris::ThreadPool::dispatch_thread() at /var/local/ldb-toolchain/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/shared_ptr_base.h:1267
        5#  doris::Thread::supervise_thread(void*) at /var/local/ldb-toolchain/bin/../usr/include/pthread.h:563
        6#  ?
        7#  ?

@Hastyshell
Copy link
Collaborator Author

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jul 28, 2025
@doris-robot
Copy link

BE UT Coverage Report

Increment line coverage 0.00% (0/5) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 57.59% (15970/27729)
Line Coverage 46.34% (143578/309837)
Region Coverage 35.77% (108312/302808)
Branch Coverage 38.30% (47781/124751)

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 28, 2025
@github-actions
Copy link
Contributor

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

@dataroaring dataroaring force-pushed the fix-clean-up-sc-null-tablet branch from 991844b to 0f1aeb9 Compare July 29, 2025 06:11
@dataroaring
Copy link
Contributor

run buildall

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17584	5571	5528	5528
q2	1941	308	196	196
q3	10278	1354	688	688
q4	10232	974	510	510
q5	8655	2324	2329	2324
q6	180	163	128	128
q7	893	784	594	594
q8	9293	1312	1079	1079
q9	6854	5065	5034	5034
q10	6937	2343	1948	1948
q11	471	278	269	269
q12	356	361	220	220
q13	17797	3501	3013	3013
q14	238	233	222	222
q15	559	465	469	465
q16	421	433	367	367
q17	579	827	379	379
q18	7581	7130	7062	7062
q19	2637	970	522	522
q20	567	315	227	227
q21	3345	3005	2355	2355
q22	1046	1037	993	993
Total cold run time: 108444 ms
Total hot run time: 34123 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5926	5767	5750	5750
q2	395	314	215	215
q3	2073	2604	2198	2198
q4	1298	1699	1315	1315
q5	4469	4421	4468	4421
q6	208	177	127	127
q7	1968	2159	2040	2040
q8	2512	2445	2403	2403
q9	7616	7298	7424	7298
q10	3115	3382	2891	2891
q11	563	506	482	482
q12	844	802	616	616
q13	3596	3691	3284	3284
q14	312	321	296	296
q15	501	467	447	447
q16	478	487	420	420
q17	1153	1439	1378	1378
q18	7902	7820	8108	7820
q19	9198	819	759	759
q20	3026	1886	1780	1780
q21	8726	4204	4250	4204
q22	1109	1011	1048	1011
Total cold run time: 66988 ms
Total hot run time: 51155 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 173276 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 0f1aeb9fea7d5cd66f32c4fd9efe2cadbbbe6b94, data reload: false

============================================
query1	1025	430	401	401
query2	6537	1744	1624	1624
query3	6741	231	223	223
query4	27203	23807	23271	23271
query5	4413	634	528	528
query6	343	247	236	236
query7	4638	511	300	300
query8	279	241	233	233
query9	8595	3148	3128	3128
query10	503	344	292	292
query11	15839	14959	14931	14931
query12	181	143	128	128
query13	1653	543	406	406
query14	9433	7710	7727	7710
query15	209	193	169	169
query16	7133	669	469	469
query17	1232	786	651	651
query18	2011	454	338	338
query19	230	224	190	190
query20	155	155	151	151
query21	218	125	110	110
query22	3934	4014	3911	3911
query23	34168	34018	34085	34018
query24	5990	2405	2432	2405
query25	515	543	473	473
query26	778	287	162	162
query27	2362	495	367	367
query28	2980	2287	2266	2266
query29	671	596	492	492
query30	290	247	204	204
query31	870	764	722	722
query32	88	83	86	83
query33	536	415	372	372
query34	810	817	520	520
query35	822	826	758	758
query36	1074	1048	951	951
query37	134	107	96	96
query38	4008	4069	3961	3961
query39	1468	1399	1372	1372
query40	233	142	133	133
query41	62	59	54	54
query42	138	126	127	126
query43	514	505	470	470
query44	1402	883	872	872
query45	203	192	184	184
query46	949	1065	677	677
query47	1831	1848	1760	1760
query48	408	429	308	308
query49	671	504	412	412
query50	670	719	426	426
query51	5417	5369	5576	5369
query52	130	133	120	120
query53	267	287	212	212
query54	676	655	556	556
query55	87	87	88	87
query56	364	360	349	349
query57	1211	1254	1137	1137
query58	361	338	328	328
query59	2549	2595	2456	2456
query60	408	390	382	382
query61	142	124	120	120
query62	763	728	659	659
query63	249	216	213	213
query64	2339	1081	790	790
query65	4214	4119	4179	4119
query66	998	463	337	337
query67	query68	18058	813	598	598
query69	1010	325	298	298
query70	1455	1161	1125	1125
query71	742	345	325	325
query72	9157	2251	2319	2251
query73	3454	642	356	356
query74	8950	9096	8689	8689
query75	7563	3162	2682	2682
query76	8824	1217	810	810
query77	1140	412	328	328
query78	9725	10629	9297	9297
query79	14859	644	590	590
query80	1312	575	484	484
query81	537	278	226	226
query82	516	164	116	116
query83	320	288	263	263
query84	294	96	80	80
query85	965	437	330	330
query86	346	336	311	311
query87	4351	4356	4212	4212
query88	5569	2274	2270	2270
query89	509	363	320	320
query90	2567	224	229	224
query91	144	139	115	115
query92	87	71	69	69
query93	6477	970	652	652
query94	1065	391	278	278
query95	430	336	317	317
query96	509	602	285	285
query97	2728	2737	2603	2603
query98	269	238	223	223
query99	1475	1418	1271	1271
Total cold run time: 302732 ms
Total hot run time: 173276 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.04
query2	0.08	0.04	0.04
query3	0.24	0.07	0.07
query4	1.62	0.10	0.11
query5	0.44	0.47	0.44
query6	1.18	0.69	0.69
query7	0.02	0.02	0.01
query8	0.04	0.03	0.03
query9	0.57	0.48	0.47
query10	0.52	0.52	0.53
query11	0.15	0.11	0.11
query12	0.15	0.11	0.11
query13	0.64	0.66	0.63
query14	1.04	1.06	0.96
query15	0.94	0.91	0.91
query16	0.38	0.41	0.39
query17	1.08	1.05	1.11
query18	0.21	0.21	0.20
query19	1.92	1.87	1.90
query20	0.02	0.02	0.02
query21	15.38	0.85	0.55
query22	0.76	1.21	0.67
query23	14.93	1.17	0.62
query24	6.69	0.75	0.57
query25	0.50	0.24	0.05
query26	0.50	0.16	0.13
query27	0.06	0.05	0.05
query28	9.98	0.88	0.45
query29	12.64	3.89	3.39
query30	3.03	2.96	2.92
query31	2.81	0.57	0.41
query32	3.23	0.58	0.49
query33	3.00	3.25	3.21
query34	16.20	5.29	4.92
query35	4.89	4.84	4.97
query36	0.69	0.52	0.50
query37	0.10	0.08	0.07
query38	0.06	0.05	0.04
query39	0.04	0.02	0.03
query40	0.17	0.14	0.14
query41	0.07	0.02	0.02
query42	0.03	0.03	0.03
query43	0.04	0.04	0.04
Total cold run time: 107.08 s
Total hot run time: 32.79 s

@doris-robot
Copy link

BE UT Coverage Report

Increment line coverage 0.00% (0/5) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 57.77% (16052/27787)
Line Coverage 46.48% (144462/310810)
Region Coverage 35.81% (108789/303799)
Branch Coverage 38.39% (48017/125075)

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 60.00% (3/5) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 81.17% (22143/27279)
Line Coverage 73.84% (229211/310395)
Region Coverage 61.68% (192147/311546)
Branch Coverage 65.34% (82647/126481)

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 60.00% (3/5) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 81.17% (22143/27279)
Line Coverage 73.84% (229211/310395)
Region Coverage 61.68% (192147/311546)
Branch Coverage 65.34% (82647/126481)

@dataroaring dataroaring merged commit 635a3dd into apache:master Jul 30, 2025
27 of 28 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 30, 2025
…ailure (#53952)

### What problem does this PR solve?

Problem Summary:

1. New tablet cloud be nullptr when get tablet action failed, take it
into account.
2. Fix grammer error of the method signature.
github-actions bot pushed a commit that referenced this pull request Jul 30, 2025
…ailure (#53952)

### What problem does this PR solve?

Problem Summary:

1. New tablet cloud be nullptr when get tablet action failed, take it
into account.
2. Fix grammer error of the method signature.
morrySnow pushed a commit that referenced this pull request Jul 30, 2025
… sc job on failure #53952 (#54065)

Cherry-picked from #53952

Co-authored-by: Siyang Tang <tangsiyang@selectdb.com>
dataroaring pushed a commit that referenced this pull request Aug 1, 2025
… sc job on failure #53952 (#54064)

Cherry-picked from #53952

Co-authored-by: Siyang Tang <tangsiyang@selectdb.com>
BiteTheDDDDt pushed a commit to BiteTheDDDDt/incubator-doris that referenced this pull request Sep 1, 2025
…ailure (apache#53952) (apache#4247)

1. New tablet cloud be nullptr when get tablet action failed, take it
into account.
2. Fix grammer error of the method signature.
@gavinchou gavinchou mentioned this pull request Sep 1, 2025
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/3.0.7-merged dev/3.1.0-merged p0_new reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants