github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4345545128
##########
be/src/runtime/exec_env.cpp:
##########
@@ -191,20 +194,44 @@ std::map<TNetworkAddress, FrontendInfo>
ExecEnv::get_running_frontends() {
return res;
}
+void ExecEnv::wait_for_shutdown_heartbeat() {
+ int32_t elapsed = 0;
+ while (!k_shutdown_heartbeat_sent && elapsed <
config::grace_shutdown_heartbeat_wait_seconds) {
+ sleep(1);
+ ++elapsed;
+ }
+ LOG(INFO) << "shutdown heartbeat prepared: " <<
k_shutdown_heartbeat_sent.load() << ", waited "
+ << elapsed << " seconds";
+ // The response may still be in transit. This buffer also gives followers
time
+ // to replay shutdown state. It does not prove that every FE has applied
it.
+ sleep(std::max(0, config::grace_shutdown_heartbeat_propagation_seconds));
+}
+
void ExecEnv::wait_for_all_tasks_done() {
- // For graceful shutdown, need to wait for all running queries to stop
+ // Both receiver implementations and the load coordinator must finish. For
example,
+ // an HTTP upload or transaction commit can outlive all of its pipeline
fragments.
int32_t wait_seconds_passed = 0;
while (true) {
int num_queries = _fragment_mgr->running_query_num();
- if (num_queries < 1) {
+ size_t num_channels = _load_channel_mgr->num_active_channels();
+ // TODO: Fix partial-open receiver cleanup (see
LoadStream::on_closed()) so
+ // abandoned entries do not consume the full
grace_shutdown_wait_seconds.
+ // Keep counting receivers until then to avoid exiting during active
loads.
+ size_t num_streams = _load_stream_mgr->get_load_stream_num();
+ size_t num_loads = StreamLoadContext::num_active_contexts();
Review Comment:
[P2] Release failed multi-table routine-load contexts before draining
This lifetime census also counts abandoned multi-table routine loads. If
`_get_or_create_query_ctx()` fails before creating a child fragment (for
example, the selected workload group is shut down between lookup and
registration), `MultiTablePipe::exec_plans()` has already incremented
`_inflight_cnt` and registered the child contexts. Its callback-taking
`FragmentMgr` overload returns without a completion callback, so
`handle_consume_finished()` cannot reach zero. `HANDLE_MULTI_TABLE_ERROR` then
waits forever at `load_status_future.get()`, and `_handle_consumer_finished()`
never unregisters the children. With no live query/channel,
`num_active_contexts()` nevertheless stays positive indefinitely, making a
later idle shutdown consume the full `grace_shutdown_wait_seconds` (120 seconds
by default). This is a separate instance from the existing transactional-INSERT
thread: this direct callback overload bypasses the new `need_txn` cleanup.
Balance the failed child-plan reservation and
release its registrations on the pre-callback error path, and cover that
multi-table failure before using all context lifetimes as drainable work.
##########
be/src/load/channel/load_channel_mgr.cpp:
##########
@@ -64,6 +64,11 @@ LoadChannelMgr::LoadChannelMgr() :
_stop_background_threads_latch(1) {
});
}
+size_t LoadChannelMgr::num_active_channels() {
+ std::lock_guard<std::mutex> lock(_lock);
Review Comment:
[P1] Keep the channel census off blocking query cleanup
This mutex can also cover synchronous filesystem cleanup.
`LoadChannelMgr::open()` holds `_lock` while constructing `LoadChannel`; the
constructor acquires a `shared_ptr<QueryContext>` for the load ID, keeps only
its `ResourceContext`, then releases the temporary owner. If a late receiver
open races completion/cancellation of a locally spilling INSERT SELECT, that
temporary can become the last owner: the query registry and resource controller
hold weak references. `QueryContext::~QueryContext()` then calls
`delete_query_spill_directory()` -> `LocalFileSystem::delete_directory_impl()`
-> `std::filesystem::remove_all()` under this manager lock. A stalled spill
filesystem therefore blocks the new shutdown poll before it can check
`grace_shutdown_wait_seconds`, even when the limit is zero. The atomic
stream-manager fix covers a different manager and does not cover this
constructor path. Make this census nonblocking, or move construction and
possible last-owner cleanup outside the mute
x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
