Uh oh!
There was an error while loading. Please reload this page.
[opt](cloud) Enable compaction on new tablets during schema change queuing - #61089
Merged
gavinchou merged 4 commits intoMar 23, 2026
Merged
Conversation
Yukang-Lian
marked this pull request as draft
March 5, 2026 15:52
Yukang-Lianforce-pushed
the
feature/cloud-sc-compaction-optimization
branch
from
March 9, 2026 03:30
c160456 to
3f1b5b1CompareYukang-Lian
marked this pull request as ready for review
March 9, 2026 16:09
Yukang-Lian
commented
Mar 10, 2026
CollaboratorAuthor
run buildall |
doris-robot
commented
Mar 10, 2026
TPC-H: Total hot run time: 27666 ms |
doris-robot
commented
Mar 10, 2026
TPC-DS: Total hot run time: 153061 ms |
Yukang-Lian
commented
Mar 11, 2026
CollaboratorAuthor
run p0 |
Yukang-Lian
commented
Mar 11, 2026
CollaboratorAuthor
run beut |
doris-robot
commented
Mar 11, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 11, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 11, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Yukang-Lian
commented
Mar 19, 2026
CollaboratorAuthor
run buildall |
doris-robot
commented
Mar 19, 2026
TPC-H: Total hot run time: 26962 ms |
doris-robot
commented
Mar 19, 2026
TPC-DS: Total hot run time: 168330 ms |
hello-stephen
commented
Mar 19, 2026
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 19, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
luwei16
approved these changes
Mar 23, 2026
Contributor
PR approved by at least one committer and no changes requested. |
Contributor
PR approved by anyone and no changes requested. |
Yukang-Lian
commented
Mar 23, 2026
CollaboratorAuthor
run external |
hello-stephen
commented
Mar 23, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
gavinchou
approved these changes
Mar 23, 2026
Uh oh!
There was an error while loading. Please reload this page.
github-actionsBot
pushed a commit
that referenced
this pull request
Mar 23, 2026
…euing (#61089) ## Summary In cloud mode, schema change (SC) tasks for multi-tablet tables are submitted to a thread pool with limited workers. Tablets waiting in the queue cannot do compaction because their `alter_version` is unset (-1), which causes rowset accumulation during long queue waits. This PR: 1. **Sets `alter_version = V0` before enqueue** via `pre_submit_callback` on `TaskWorkerPool`, allowing compaction on new tablets for versions > V0 during the queue wait period. 2. **Fixes a cross-V1 compaction race condition**: During the queue wait, compaction may commit a rowset that crosses the eventual V1 boundary (determined later by `prepare_tablet_job`). After SC commit, this creates version overlap and causes BE crash (`Check failed: versions are not continuity`). The fix adds a cross-V1 check in `process_alter_tablet` after `prepare_tablet_job` returns V1: sync new tablet and reject if any rowset crosses V1, letting FE retry with a higher V1. ## Changes **BE only — no FE or meta-service changes.** ### Core logic - `task_worker_pool.h/cpp`: Add optional `pre_submit_callback` to `TaskWorkerPool`, called synchronously before task enqueue. - `agent_server.cpp`: Pass `set_alter_version_before_enqueue` as `pre_submit_callback` for ALTER workers. - `task_worker_pool.cpp`: Implement `set_alter_version_before_enqueue` (sets alter_version=V0 on both tablets) and cleanup logic in `alter_cloud_tablet_callback` (resets alter_version=-1 before `remove_task_info` to avoid race with re-submission). - `cloud_schema_change_job.cpp`: Add cross-V1 compaction check after `prepare_tablet_job` — sync new tablet and verify no rowset crosses V1 boundary. If detected, return error for FE retry (next retry V1 will be higher, cross-boundary rowset falls within [2, V1']). ### Key Design Points - **V0** (partition visible version from FE) is set at pre-submit time to enable compaction during queue wait. - **V1** (base_max_version) is still determined at SC execution time for rowset conversion alignment — unchanged behavior. - SC commit atomically replaces rowsets [2, V1] on the new tablet, so any compaction results in (V0, V1] during queue time are harmlessly discarded. - Cleanup order: `alter_version` is reset to -1 before `remove_task_info` to prevent race with same-signature re-submission. ### Cross-V1 race condition analysis The dangerous timing window is: compaction commits to meta-service BEFORE `prepare_tablet_job` registers the SC job. At that point, no `JOB_CHECK_ALTER_VERSION` protection exists, so compaction succeeds. When SC later commits with a lower V1, meta-service cannot delete the cross-V1 rowset, creating version overlap. After `prepare_tablet_job`, two existing mechanisms protect: - `clear_compaction`: `prepare_tablet_job` clears all compaction jobs on new tablet, so in-flight compaction commits are rejected. - `JOB_CHECK_ALTER_VERSION`: new compaction starts are rejected if input versions cross V1. The fix covers the only unprotected window by checking for cross-V1 rowsets right after `prepare_tablet_job` returns. ### Tests - `task_worker_pool_test.cpp`: UT for `pre_submit_callback` mechanism and task dedup behavior. - `test_sc_compaction_optimization.groovy`: Verify compaction runs on new tablets during SC queue wait (merged rowset version range proves compaction happened). - `test_sc_compaction_optimization_with_load.groovy`: End-to-end with concurrent writes, data consistency after SC + compaction. - `test_sc_compaction_cross_v1_race.groovy`: Reproduce cross-V1 race using `override_base_max_version` debug point. Without fix: BE crash. With fix: SC detects cross-V1 and returns retryable error, BE stays alive.
github-actionsBot
pushed a commit
that referenced
this pull request
Mar 23, 2026
…euing (#61089) ## Summary In cloud mode, schema change (SC) tasks for multi-tablet tables are submitted to a thread pool with limited workers. Tablets waiting in the queue cannot do compaction because their `alter_version` is unset (-1), which causes rowset accumulation during long queue waits. This PR: 1. **Sets `alter_version = V0` before enqueue** via `pre_submit_callback` on `TaskWorkerPool`, allowing compaction on new tablets for versions > V0 during the queue wait period. 2. **Fixes a cross-V1 compaction race condition**: During the queue wait, compaction may commit a rowset that crosses the eventual V1 boundary (determined later by `prepare_tablet_job`). After SC commit, this creates version overlap and causes BE crash (`Check failed: versions are not continuity`). The fix adds a cross-V1 check in `process_alter_tablet` after `prepare_tablet_job` returns V1: sync new tablet and reject if any rowset crosses V1, letting FE retry with a higher V1. ## Changes **BE only — no FE or meta-service changes.** ### Core logic - `task_worker_pool.h/cpp`: Add optional `pre_submit_callback` to `TaskWorkerPool`, called synchronously before task enqueue. - `agent_server.cpp`: Pass `set_alter_version_before_enqueue` as `pre_submit_callback` for ALTER workers. - `task_worker_pool.cpp`: Implement `set_alter_version_before_enqueue` (sets alter_version=V0 on both tablets) and cleanup logic in `alter_cloud_tablet_callback` (resets alter_version=-1 before `remove_task_info` to avoid race with re-submission). - `cloud_schema_change_job.cpp`: Add cross-V1 compaction check after `prepare_tablet_job` — sync new tablet and verify no rowset crosses V1 boundary. If detected, return error for FE retry (next retry V1 will be higher, cross-boundary rowset falls within [2, V1']). ### Key Design Points - **V0** (partition visible version from FE) is set at pre-submit time to enable compaction during queue wait. - **V1** (base_max_version) is still determined at SC execution time for rowset conversion alignment — unchanged behavior. - SC commit atomically replaces rowsets [2, V1] on the new tablet, so any compaction results in (V0, V1] during queue time are harmlessly discarded. - Cleanup order: `alter_version` is reset to -1 before `remove_task_info` to prevent race with same-signature re-submission. ### Cross-V1 race condition analysis The dangerous timing window is: compaction commits to meta-service BEFORE `prepare_tablet_job` registers the SC job. At that point, no `JOB_CHECK_ALTER_VERSION` protection exists, so compaction succeeds. When SC later commits with a lower V1, meta-service cannot delete the cross-V1 rowset, creating version overlap. After `prepare_tablet_job`, two existing mechanisms protect: - `clear_compaction`: `prepare_tablet_job` clears all compaction jobs on new tablet, so in-flight compaction commits are rejected. - `JOB_CHECK_ALTER_VERSION`: new compaction starts are rejected if input versions cross V1. The fix covers the only unprotected window by checking for cross-V1 rowsets right after `prepare_tablet_job` returns. ### Tests - `task_worker_pool_test.cpp`: UT for `pre_submit_callback` mechanism and task dedup behavior. - `test_sc_compaction_optimization.groovy`: Verify compaction runs on new tablets during SC queue wait (merged rowset version range proves compaction happened). - `test_sc_compaction_optimization_with_load.groovy`: End-to-end with concurrent writes, data consistency after SC + compaction. - `test_sc_compaction_cross_v1_race.groovy`: Reproduce cross-V1 race using `override_base_max_version` debug point. Without fix: BE crash. With fix: SC detects cross-V1 and returns retryable error, BE stays alive.
HappenLee pushed a commit
to HappenLee/incubator-doris
that referenced
this pull request
Mar 31, 2026
…euing (apache#61089) ## Summary In cloud mode, schema change (SC) tasks for multi-tablet tables are submitted to a thread pool with limited workers. Tablets waiting in the queue cannot do compaction because their `alter_version` is unset (-1), which causes rowset accumulation during long queue waits. This PR: 1. **Sets `alter_version = V0` before enqueue** via `pre_submit_callback` on `TaskWorkerPool`, allowing compaction on new tablets for versions > V0 during the queue wait period. 2. **Fixes a cross-V1 compaction race condition**: During the queue wait, compaction may commit a rowset that crosses the eventual V1 boundary (determined later by `prepare_tablet_job`). After SC commit, this creates version overlap and causes BE crash (`Check failed: versions are not continuity`). The fix adds a cross-V1 check in `process_alter_tablet` after `prepare_tablet_job` returns V1: sync new tablet and reject if any rowset crosses V1, letting FE retry with a higher V1. ## Changes **BE only — no FE or meta-service changes.** ### Core logic - `task_worker_pool.h/cpp`: Add optional `pre_submit_callback` to `TaskWorkerPool`, called synchronously before task enqueue. - `agent_server.cpp`: Pass `set_alter_version_before_enqueue` as `pre_submit_callback` for ALTER workers. - `task_worker_pool.cpp`: Implement `set_alter_version_before_enqueue` (sets alter_version=V0 on both tablets) and cleanup logic in `alter_cloud_tablet_callback` (resets alter_version=-1 before `remove_task_info` to avoid race with re-submission). - `cloud_schema_change_job.cpp`: Add cross-V1 compaction check after `prepare_tablet_job` — sync new tablet and verify no rowset crosses V1 boundary. If detected, return error for FE retry (next retry V1 will be higher, cross-boundary rowset falls within [2, V1']). ### Key Design Points - **V0** (partition visible version from FE) is set at pre-submit time to enable compaction during queue wait. - **V1** (base_max_version) is still determined at SC execution time for rowset conversion alignment — unchanged behavior. - SC commit atomically replaces rowsets [2, V1] on the new tablet, so any compaction results in (V0, V1] during queue time are harmlessly discarded. - Cleanup order: `alter_version` is reset to -1 before `remove_task_info` to prevent race with same-signature re-submission. ### Cross-V1 race condition analysis The dangerous timing window is: compaction commits to meta-service BEFORE `prepare_tablet_job` registers the SC job. At that point, no `JOB_CHECK_ALTER_VERSION` protection exists, so compaction succeeds. When SC later commits with a lower V1, meta-service cannot delete the cross-V1 rowset, creating version overlap. After `prepare_tablet_job`, two existing mechanisms protect: - `clear_compaction`: `prepare_tablet_job` clears all compaction jobs on new tablet, so in-flight compaction commits are rejected. - `JOB_CHECK_ALTER_VERSION`: new compaction starts are rejected if input versions cross V1. The fix covers the only unprotected window by checking for cross-V1 rowsets right after `prepare_tablet_job` returns. ### Tests - `task_worker_pool_test.cpp`: UT for `pre_submit_callback` mechanism and task dedup behavior. - `test_sc_compaction_optimization.groovy`: Verify compaction runs on new tablets during SC queue wait (merged rowset version range proves compaction happened). - `test_sc_compaction_optimization_with_load.groovy`: End-to-end with concurrent writes, data consistency after SC + compaction. - `test_sc_compaction_cross_v1_race.groovy`: Reproduce cross-V1 race using `override_base_max_version` debug point. Without fix: BE crash. With fix: SC detects cross-V1 and returns retryable error, BE stays alive.
14 tasks
liaoxin01 pushed a commit
that referenced
this pull request
Apr 10, 2026
…change (#62256) ### What problem does this PR solve? Problem Summary: During cloud schema change, the MS (Meta Service) side correctly recycles rowsets in `[2, alter_version]` on the new tablet when committing the SC job. However, the BE side did not mirror this behavior — it directly called `add_rowsets` for the SC output without first removing existing local rowsets. This could leave stale rowsets (e.g., compaction outputs on the new tablet) visible in `_rs_version_map`, and since their delete bitmap does not cover the SC output rows, duplicate keys may appear in MOW tables. PR #61089 increased the likelihood of triggering this issue by enabling compaction on new tablets during SC, which makes it more common for the new tablet to have compaction rowsets with wider version ranges (e.g., `[818-822]`) that overlap with individual SC output rowsets (e.g., `[818],[819],...,[822]`). The `add_rowsets` overlap check (`to_add_v.contains(v)`) is one-directional: `[818].contains([818-822])` evaluates to false, so the stale compaction rowset was not removed. Fix: Before calling `add_rowsets` for SC output, delete all local rowsets in `[2, alter_version]` from the new tablet, mirroring the MS-side recycle behavior. A new `CloudTablet::delete_rowsets_for_schema_change` method is added that also removes edges from the version graph, preventing the greedy capture algorithm from preferring the wider stale compaction path over the individual SC output rowsets.
github-actionsBot
pushed a commit
that referenced
this pull request
Apr 10, 2026
…change (#62256) ### What problem does this PR solve? Problem Summary: During cloud schema change, the MS (Meta Service) side correctly recycles rowsets in `[2, alter_version]` on the new tablet when committing the SC job. However, the BE side did not mirror this behavior — it directly called `add_rowsets` for the SC output without first removing existing local rowsets. This could leave stale rowsets (e.g., compaction outputs on the new tablet) visible in `_rs_version_map`, and since their delete bitmap does not cover the SC output rows, duplicate keys may appear in MOW tables. PR #61089 increased the likelihood of triggering this issue by enabling compaction on new tablets during SC, which makes it more common for the new tablet to have compaction rowsets with wider version ranges (e.g., `[818-822]`) that overlap with individual SC output rowsets (e.g., `[818],[819],...,[822]`). The `add_rowsets` overlap check (`to_add_v.contains(v)`) is one-directional: `[818].contains([818-822])` evaluates to false, so the stale compaction rowset was not removed. Fix: Before calling `add_rowsets` for SC output, delete all local rowsets in `[2, alter_version]` from the new tablet, mirroring the MS-side recycle behavior. A new `CloudTablet::delete_rowsets_for_schema_change` method is added that also removes edges from the version graph, preventing the greedy capture algorithm from preferring the wider stale compaction path over the individual SC output rowsets.
github-actionsBot
pushed a commit
that referenced
this pull request
Apr 10, 2026
…change (#62256) ### What problem does this PR solve? Problem Summary: During cloud schema change, the MS (Meta Service) side correctly recycles rowsets in `[2, alter_version]` on the new tablet when committing the SC job. However, the BE side did not mirror this behavior — it directly called `add_rowsets` for the SC output without first removing existing local rowsets. This could leave stale rowsets (e.g., compaction outputs on the new tablet) visible in `_rs_version_map`, and since their delete bitmap does not cover the SC output rows, duplicate keys may appear in MOW tables. PR #61089 increased the likelihood of triggering this issue by enabling compaction on new tablets during SC, which makes it more common for the new tablet to have compaction rowsets with wider version ranges (e.g., `[818-822]`) that overlap with individual SC output rowsets (e.g., `[818],[819],...,[822]`). The `add_rowsets` overlap check (`to_add_v.contains(v)`) is one-directional: `[818].contains([818-822])` evaluates to false, so the stale compaction rowset was not removed. Fix: Before calling `add_rowsets` for SC output, delete all local rowsets in `[2, alter_version]` from the new tablet, mirroring the MS-side recycle behavior. A new `CloudTablet::delete_rowsets_for_schema_change` method is added that also removes edges from the version graph, preventing the greedy capture algorithm from preferring the wider stale compaction path over the individual SC output rowsets.
bobhan1 added a commit
that referenced
this pull request
May 11, 2026
…change (#62256) ### What problem does this PR solve? Problem Summary: During cloud schema change, the MS (Meta Service) side correctly recycles rowsets in `[2, alter_version]` on the new tablet when committing the SC job. However, the BE side did not mirror this behavior — it directly called `add_rowsets` for the SC output without first removing existing local rowsets. This could leave stale rowsets (e.g., compaction outputs on the new tablet) visible in `_rs_version_map`, and since their delete bitmap does not cover the SC output rows, duplicate keys may appear in MOW tables. PR #61089 increased the likelihood of triggering this issue by enabling compaction on new tablets during SC, which makes it more common for the new tablet to have compaction rowsets with wider version ranges (e.g., `[818-822]`) that overlap with individual SC output rowsets (e.g., `[818],[819],...,[822]`). The `add_rowsets` overlap check (`to_add_v.contains(v)`) is one-directional: `[818].contains([818-822])` evaluates to false, so the stale compaction rowset was not removed. Fix: Before calling `add_rowsets` for SC output, delete all local rowsets in `[2, alter_version]` from the new tablet, mirroring the MS-side recycle behavior. A new `CloudTablet::delete_rowsets_for_schema_change` method is added that also removes edges from the version graph, preventing the greedy capture algorithm from preferring the wider stale compaction path over the individual SC output rowsets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Summary
In cloud mode, schema change (SC) tasks for multi-tablet tables are submitted to a thread pool with limited workers. Tablets waiting in the queue cannot do compaction because their
alter_versionis unset (-1), which causes rowset accumulation during long queue waits.This PR:
Sets
alter_version = V0before enqueue viapre_submit_callbackonTaskWorkerPool, allowing compaction on new tablets for versions > V0 during the queue wait period.Fixes a cross-V1 compaction race condition: During the queue wait, compaction may commit a rowset that crosses the eventual V1 boundary (determined later by
prepare_tablet_job). After SC commit, this creates version overlap and causes BE crash (Check failed: versions are not continuity). The fix adds a cross-V1 check inprocess_alter_tabletafterprepare_tablet_jobreturns V1: sync new tablet and reject if any rowset crosses V1, letting FE retry with a higher V1.Changes
BE only — no FE or meta-service changes.
Core logic
task_worker_pool.h/cpp: Add optionalpre_submit_callbacktoTaskWorkerPool, called synchronously before task enqueue.agent_server.cpp: Passset_alter_version_before_enqueueaspre_submit_callbackfor ALTER workers.task_worker_pool.cpp: Implementset_alter_version_before_enqueue(sets alter_version=V0 on both tablets) and cleanup logic inalter_cloud_tablet_callback(resets alter_version=-1 beforeremove_task_infoto avoid race with re-submission).cloud_schema_change_job.cpp: Add cross-V1 compaction check afterprepare_tablet_job— sync new tablet and verify no rowset crosses V1 boundary. If detected, return error for FE retry (next retry V1 will be higher, cross-boundary rowset falls within [2, V1']).Key Design Points
alter_versionis reset to -1 beforeremove_task_infoto prevent race with same-signature re-submission.Cross-V1 race condition analysis
The dangerous timing window is: compaction commits to meta-service BEFORE
prepare_tablet_jobregisters the SC job. At that point, noJOB_CHECK_ALTER_VERSIONprotection exists, so compaction succeeds. When SC later commits with a lower V1, meta-service cannot delete the cross-V1 rowset, creating version overlap.After
prepare_tablet_job, two existing mechanisms protect:clear_compaction:prepare_tablet_jobclears all compaction jobs on new tablet, so in-flight compaction commits are rejected.JOB_CHECK_ALTER_VERSION: new compaction starts are rejected if input versions cross V1.The fix covers the only unprotected window by checking for cross-V1 rowsets right after
prepare_tablet_jobreturns.Tests
task_worker_pool_test.cpp: UT forpre_submit_callbackmechanism and task dedup behavior.test_sc_compaction_optimization.groovy: Verify compaction runs on new tablets during SC queue wait (merged rowset version range proves compaction happened).test_sc_compaction_optimization_with_load.groovy: End-to-end with concurrent writes, data consistency after SC + compaction.test_sc_compaction_cross_v1_race.groovy: Reproduce cross-V1 race usingoverride_base_max_versiondebug point. Without fix: BE crash. With fix: SC detects cross-V1 and returns retryable error, BE stays alive.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)