Conversation
### What problem does this PR solve?
When load writes, flush waits, or closes occupy the shared BRPC heavy pool, `tablet_writer_open`, `tablet_writer_cancel`, and `open_load_stream` queue behind that work. In particular, a cancellation intended to release load resources cannot be dispatched promptly under heavy-pool saturation.
Add two dedicated load pools:
- `brpc_load_light`: writer open/cancel and stream open.
- `brpc_load_heavy`: add-block RPCs (including the HTTP forwarding path) and `LoadStreamMgr` flush/pre-close/close tasks.
Both pools have independent thread/queue settings and queue-size, active-thread, effective thread-limit, and effective queue-limit metrics. The four new `brpc_load_{heavy,light}_work_pool_{threads,max_queue_size}` settings require a restart and accept `-1` or a positive value. By default, load-heavy inherits the existing heavy-pool settings; load-light uses `max(32, CPU cores)` threads and `max(1024, CPU cores * 32)` queued requests.
The split reserves execution capacity for load control requests. It does not remove locks or storage operations inside open/cancel, and it creates additional fixed worker threads. Existing generic heavy/light pools, RPC response contracts, and storage/transaction semantics are retained. No throughput or latency benchmark is claimed.
### Release note
Isolate load open and cancellation RPCs from load write/close queues to reduce control-request queueing under load. Add independently configurable load-heavy and load-light BRPC worker pools and metrics.
### Check List (For Author)
- Test
- [x] Unit Test: added deterministic queue-routing and queue-full callback/status coverage for writer open/cancel, stream open, and add-block, plus streaming close-pool wiring. **Not executed**, as requested by the author.
- Validation completed: clang-format 16.0.5 on all five changed C++ files and `git diff --check`.
- Build/UT execution skipped at the author's request. An initial build-environment probe was stopped during third-party dependency setup; no BE build or test result is claimed.
- Behavior changed:
- [x] Yes. Load control and data requests use separate pools; existing generic RPC pools retain their configuration names.
- Does this need documentation?
- [x] No separate documentation change in this PR. New tuning defaults and restart requirements are documented alongside the BE configuration declarations.
### Review notes
- Concurrency: only dispatch destinations change; handlers retain their existing locks and memory-tracking contexts. The new pools introduce no new lock order.
- Lifetime: pools remain owned by `PInternalService`; `LoadStreamMgr` borrows its load-heavy pool using the existing ownership model. New metric hooks are deregistered on destruction.
- Compatibility: no protobuf, persistent format, FE variable, visibility, or transaction protocol change.
- Parallel paths: HTTP add-block delegates to the modified RPC; streaming open and flush/close are included; shared service dispatch covers cloud and local storage modes.
- Failure handling: queue rejection reports the destination pool and preserves exactly-once completion behavior, including the empty cancellation response.
- Coverage limit: the added tests cover dispatch/backpressure and wiring; end-to-end load behavior and performance were not exercised.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### What problem does this PR solve? Problem Summary: Keep load open/cancel isolated with one dedicated load-light pool, while retaining the existing heavy pool for writes and streaming flush/close. Remove the redundant load-heavy pool and its settings and metrics, reducing the additional fixed worker count. Update dispatch and backpressure test coverage. ### Release note Only load control RPCs use the new load-light pool. Existing heavy-pool settings and metrics continue to apply to load data work. ### Check List (For Author) - Test: Updated unit tests; compilation and test execution skipped at user request. Formatting, build header hygiene, and git diff --check passed. - Behavior changed: Yes; load data work uses the original heavy pool. - Does this need documentation: No; configuration comments describe the new pool.
sollhui
changed the title
### What problem does this PR solve? Problem Summary: Open requests can block on locks or metadata RPCs and exhaust workers shared with cancellation. Route only tablet_writer_cancel to load-light, restore both open handlers to the existing heavy pool, and fix the cancellation worker count at 32. Keep queue capacity configurable and update metrics and tests. ### Release note Only tablet writer cancellation uses the dedicated 32-thread pool. Load open, write, and close continue using the existing heavy pool. ### Check List (For Author) - Test: Updated unit tests; compilation and execution skipped at user request. Clang-format 16, build header hygiene, and git diff --check passed. - Behavior changed: Yes; only cancellation moves to a fixed 32-thread pool. - Does this need documentation: No; code/config comments describe the settings.
sollhui
changed the title
sollhui
changed the title
### What problem does this PR solve? Problem Summary: Replace the hard-coded cancellation worker count with brpc_load_light_work_pool_threads, defaulting to 32. Validate that the value is positive, use it when constructing the pool, and expose it in the capacity metric. Update unit test coverage to use a non-default worker count. ### Release note Allow configuring the cancellation pool worker count, default 32. A BE restart is required for changes to take effect. ### Check List (For Author) - Test: Unit tests updated but not run; compilation skipped at user request. Clang-format 16, header hygiene, and git diff --check passed. - Behavior changed: Yes; cancellation worker count is configurable. - Does this need documentation: No separate change; defaults and restart requirements are documented in configuration comments.
Contributor
Author
|
run buildall |
sollhui
changed the title
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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?
tablet_writer_cancelshares the BRPC heavy work pool with load open, write, and close requests. When these operations saturate the pool, cancellation requests also queue behind them, delaying cancellation and resource release.Dispatch
tablet_writer_cancelthrough a dedicatedbrpc_load_lightpool. Writer open and stream open remain in the heavy pool because they can block on locks or metadata RPCs; writes and closes also keep their existing scheduling. This isolates cancellation dispatch from heavy-pool congestion, while cancellation still follows the existing locking and cleanup logic.The new pool adds queue-size, active-thread, and capacity metrics.
brpc_load_light_work_pool_threadscontrols its worker count, defaults to32, and accepts positive values. Its queue capacity is controlled bybrpc_load_light_work_pool_max_queue_size:-1selectsmax(1024, CPU cores * 32), and a positive value sets an explicit limit. Both settings require a BE restart.Release note
Isolate tablet writer cancellation RPCs in a dedicated thread pool to reduce cancellation delays caused by a saturated BRPC heavy work pool.
Check List (For Author)
git diff --check.