-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
[Enhancement] Move delta write open out of brpc worker #56517
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,23 +85,60 @@ LoadChannelMgr::~LoadChannelMgr() { | |
} | ||
|
||
void LoadChannelMgr::close() { | ||
std::lock_guard l(_lock); | ||
for (auto iter = _load_channels.begin(); iter != _load_channels.end();) { | ||
iter->second->cancel(); | ||
iter->second->abort(); | ||
iter = _load_channels.erase(iter); | ||
{ | ||
std::lock_guard l(_lock); | ||
for (auto iter = _load_channels.begin(); iter != _load_channels.end();) { | ||
iter->second->cancel(); | ||
iter->second->abort(); | ||
iter = _load_channels.erase(iter); | ||
} | ||
} | ||
if (_async_rpc_pool) { | ||
_async_rpc_pool->shutdown(); | ||
} | ||
} | ||
|
||
Status LoadChannelMgr::init(MemTracker* mem_tracker) { | ||
_mem_tracker = mem_tracker; | ||
RETURN_IF_ERROR(_start_bg_worker()); | ||
int num_threads = config::load_channel_rpc_thread_pool_num; | ||
if (num_threads <= 0) { | ||
num_threads = CpuInfo::num_cores(); | ||
} | ||
RETURN_IF_ERROR(ThreadPoolBuilder("load_channel_mgr") | ||
.set_min_threads(5) | ||
.set_max_threads(num_threads) | ||
.set_max_queue_size(config::load_channel_rpc_thread_pool_queue_size) | ||
.set_idle_timeout(MonoDelta::FromMilliseconds(10000)) | ||
.build(&_async_rpc_pool)); | ||
REGISTER_THREAD_POOL_METRICS(load_channel_mgr, _async_rpc_pool.get()); | ||
return Status::OK(); | ||
} | ||
|
||
void LoadChannelMgr::open(brpc::Controller* cntl, const PTabletWriterOpenRequest& request, | ||
PTabletWriterOpenResult* response, google::protobuf::Closure* done) { | ||
ClosureGuard done_guard(done); | ||
LoadChannelOpenRequest open_request; | ||
open_request.cntl = cntl; | ||
open_request.request = &request; | ||
open_request.response = response; | ||
open_request.done = done; | ||
open_request.receive_rpc_time_ns = MonotonicNanos(); | ||
if (!config::enable_load_channel_rpc_async) { | ||
_open(open_request); | ||
return; | ||
} | ||
auto task = [=]() { this->_open(open_request); }; | ||
Status status = _async_rpc_pool->submit_func(std::move(task)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the task can be submit successful but not executed at all when doing pool shutdown, hence lead the |
||
if (!status.ok()) { | ||
ClosureGuard closure_guard(done); | ||
status.to_protobuf(response->mutable_status()); | ||
} | ||
} | ||
|
||
void LoadChannelMgr::_open(LoadChannelOpenRequest open_request) { | ||
ClosureGuard done_guard(open_request.done); | ||
const PTabletWriterOpenRequest& request = *open_request.request; | ||
PTabletWriterOpenResult* response = open_request.response; | ||
if (!request.encryption_meta().empty()) { | ||
Status st = KeyCache::instance().refresh_keys(request.encryption_meta()); | ||
if (!st.ok()) { | ||
|
@@ -143,7 +180,8 @@ void LoadChannelMgr::open(brpc::Controller* cntl, const PTabletWriterOpenRequest | |
return; | ||
} | ||
} | ||
channel->open(cntl, request, response, done_guard.release()); | ||
done_guard.release(); | ||
channel->open(open_request); | ||
} | ||
|
||
void LoadChannelMgr::add_chunk(const PTabletWriterAddChunkRequest& request, PTabletWriterAddBatchResult* response) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -205,6 +205,7 @@ class StarRocksMetrics { | |
METRIC_DEFINE_INT_COUNTER(load_bytes_total, MetricUnit::BYTES); | ||
|
||
// Metrics for LoadChannel | ||
METRICS_DEFINE_THREAD_POOL(load_channel_mgr); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. load_channel_pool? a little confused a thread pool is named as *_mgr |
||
// The number that LoadChannel#add_chunks is accessed | ||
METRIC_DEFINE_INT_COUNTER(load_channel_add_chunks_total, MetricUnit::OPERATIONS); | ||
// The number that LoadChannel#add_chunks eos is accessed | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also the same as the RPC's controller, maybe name it
LoadChannelOpenController
orLoadChannelOpenContext