Uh oh!
There was an error while loading. Please reload this page.
[refactor](compaction) submit manual full compaction task to thread pool instead of detached thread - #61222
Conversation
Yukang-Lian
commented
Mar 11, 2026
run buildall |
Thearas
commented
Mar 11, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Yukang-Lian
commented
Mar 11, 2026
run buildall |
1 similar comment
Yukang-Lian
commented
Mar 11, 2026
run buildall |
doris-robot
commented
Mar 11, 2026
TPC-H: Total hot run time: 27717 ms |
doris-robot
commented
Mar 11, 2026
TPC-DS: Total hot run time: 153336 ms |
doris-robot
commented
Mar 11, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 11, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
002ec48 to
7ff50aeCompareYukang-Lian
commented
Mar 12, 2026
run buildall |
1 similar comment
Yukang-Lian
commented
Mar 12, 2026
run buildall |
doris-robot
commented
Mar 12, 2026
TPC-H: Total hot run time: 27766 ms |
doris-robot
commented
Mar 12, 2026
TPC-DS: Total hot run time: 153332 ms |
doris-robot
commented
Mar 12, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 12, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Yukang-Lian
commented
Mar 13, 2026
run p0 |
Yukang-Lian
commented
Mar 13, 2026
run nonConcurrent |
Yukang-Lian
commented
Mar 13, 2026
run p0 |
Yukang-Lian
commented
Mar 13, 2026
run nonConcurrent |
hello-stephen
commented
Mar 13, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 13, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
7ff50ae to
827471cCompareYukang-Lian
commented
Mar 16, 2026
run buildall |
doris-robot
commented
Mar 16, 2026
TPC-H: Total hot run time: 27084 ms |
doris-robot
commented
Mar 16, 2026
TPC-DS: Total hot run time: 168774 ms |
hello-stephen
commented
Mar 16, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 16, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Yukang-Lian
commented
Mar 17, 2026
run buildall |
doris-robot
commented
Mar 17, 2026
TPC-H: Total hot run time: 26743 ms |
hello-stephen
commented
Apr 9, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
4de9c7c to
522bad5CompareYukang-Lian
commented
Apr 14, 2026
run buildall |
hello-stephen
commented
Apr 14, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 14, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Yukang-Lian
commented
Apr 16, 2026
run nonconcurrent |
Yukang-Lian
commented
Apr 16, 2026
/review |
There was a problem hiding this comment.
I found blocking issues in the new single-tablet full-compaction path.
- The handler now calls
StorageEngine::submit_compaction_task()synchronously. On the local engine that helper runsFullCompaction::prepare_compact()on the HTTP thread, andprepare_compact()blocks on the base/cumulative compaction locks. If another compaction is already running,/api/compaction/run?tablet_id=...&compact_type=fullcan now hang until it finishes, whereas the old detached-thread path returned after at most 2 seconds. - The new path also inherits a latent submit-status bug in
StorageEngine::_submit_compaction_task(): afterthread_pool->submit_func(...), the code checksstinstead ofstatus, so enqueue failures are silently treated as success and leak the submitted-state bookkeeping. Routing single-tablet full compaction through this helper makes that behavior user-visible.
Critical Checkpoints
- Goal / correctness: The goal is to move single-tablet manual full compaction onto the managed thread-pool path and add
force. The refactor is incomplete because the new path changes handler responsiveness and can falsely report success on enqueue failure. - Minimality / focus: The code change is focused, but it pulls in existing
submit_compaction_task()semantics that do not match the old single-tablet API contract. - Concurrency: Applicable. The change crosses the HTTP worker thread, the base-compaction pool, and the tablet compaction locks. Lock acquisition for full compaction is no longer offloaded from the request thread, which creates a regression.
- Lifecycle / static init: No special lifecycle or static-initialization issue found in the patch itself.
- Configs: No new configuration items.
- Parallel paths: Applicable. Table-level local full compaction already uses the thread-pool path, but the old single-tablet path had different responsiveness and error semantics. The patch aligns implementations without preserving those semantics.
- Conditions / special checks: The new
forceparsing is straightforward; no issue there. - Tests: Coverage is insufficient for the changed behavior. The added UTs only cover parameter parsing / not-found cases, not actual thread-pool submission, lock contention, or enqueue-failure behavior.
- Observability: The thread-pool path keeps metrics/tracker integration, which is good.
- Transaction / persistence: Not applicable.
- Data writes / atomicity: No new storage-format or persistence change reviewed here.
- FE/BE variable passing: Not applicable.
- Performance / resource usage: Blocking the HTTP worker on compaction locks is a regression.
- Other issues: No additional blocking issues beyond the inline comments.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Yukang-Lian
commented
Apr 17, 2026
run nonconcurrent |
hello-stephen
commented
Apr 17, 2026
run nonConcurrent |
hello-stephen
commented
Apr 17, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Uh oh!
There was an error while loading. Please reload this page.
PR approved by at least one committer and no changes requested. |
Cherry-pick the following PRs to `branch-4.1`: - #61696 [Feature](compaction) add CompactionTaskTracker with system table and HTTP API - #61621 [fix](metrics) Fix prepared statement QPS metrics not counted when audit log disabled - #61222 [refactor](compaction) submit manual full compaction task to thread pool instead of detached thread - #62596 [feat](compaction) Support ADMIN COMPACT TABLE type='FULL' and enable it in cloud mode Conflicts resolved for #61696 (schema table enum renumbering on branch-4.1; `be_compaction_tasks` uses id 67) and #61621 (AuditLogHelper context + QueueToken import).
…ool instead of detached thread (apache#61222) Previously, manually triggering full compaction on a single tablet via HTTP API would create a detached thread for each request, which lacks concurrency control, deduplication, and metrics tracking. This PR changes the single-tablet full compaction path to use `submit_compaction_task()`, submitting to the base compaction thread pool — consistent with the table-level path and the cloud engine. Base and cumulative compaction remain unchanged. Added `force` HTTP parameter to skip permit limiter when submitting full compaction, allowing compaction to proceed even when resources are constrained. ### Usage ```bash # single tablet, default (with permit limiter) curl -X POST "http://be_host:http_port/api/compaction/run?tablet_id=12345&compact_type=full" # single tablet, force (skip permit limiter) curl -X POST "http://be_host:http_port/api/compaction/run?tablet_id=12345&compact_type=full&force=true" # table level curl -X POST "http://be_host:http_port/api/compaction/run?table_id=67890&compact_type=full" # table level, force curl -X POST "http://be_host:http_port/api/compaction/run?table_id=67890&compact_type=full&force=true" ```
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Previously, manually triggering full compaction on a single tablet via HTTP API would create a detached thread for each request, which lacks concurrency control, deduplication, and metrics tracking.
This PR changes the single-tablet full compaction path to use
submit_compaction_task(), submitting to the base compaction thread pool — consistent with the table-level path and the cloud engine. Base and cumulative compaction remain unchanged.Added
forceHTTP parameter to skip permit limiter when submitting full compaction, allowing compaction to proceed even when resources are constrained.Usage
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)