github-actions[bot] commented on code in PR #69631:
URL: https://github.com/apache/doris/pull/69631#discussion_r4376412528
##########
fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/results/FlightSqlChannel.java:
##########
@@ -156,6 +156,12 @@ public void invalidate(String handle) {
resultCache.invalidate(handle);
}
+ public boolean hasPendingResults() {
+ // Expired local tickets no longer need to hold up shutdown.
+ resultCache.cleanUp();
Review Comment:
[P2] Pin Flight results before expiring them during shutdown
This new shutdown-thread cleanup can close a result already borrowed by an
admitted `DoGet`. For a ticket fetched just before its 10-minute expiry,
`getStreamStatementResult()` can obtain the cached `VectorSchemaRoot`, then
this poll expires the entry and its removal listener closes the root before
`listener.start()/putNext()` finishes reading it. The active-request count
keeps shutdown waiting but does not protect those buffers, so a previously
valid retrieval can fail during serialization. In [Arrow 19, the listener keeps
the root and unloads its buffers
later](https://github.com/apache/arrow-java/blob/v19.0.0/flight/flight-core/src/main/java/org/apache/arrow/flight/OutboundStreamListenerImpl.java#L49-L95).
Pin/reference-count in-use entries or serialize removal against borrowing and
batch unloading; the drain poll must not free an active reader's vectors.
##########
be/src/runtime/exec_env.cpp:
##########
@@ -170,20 +174,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();
Review Comment:
[P2] Reserve queued opens for the legacy receiver too
The new census can still report zero with an accepted `tablet_writer_open`
waiting in `_heavy_work_pool`: that RPC has no `PendingOpen` reservation, and
`LoadChannelMgr::open()` inserts its channel only after the worker starts. A
remote load receiver can have neither a local fragment nor a
`StreamLoadContext` (the `LoadChannel` constructor explicitly supports that
case). If the worker starts after this census, the load runs only during the
fixed post-delay and can be killed while `grace_shutdown_wait_seconds` remains
unused. The new reservation in `open_load_stream` covers the other receiver
implementation only. Reserve legacy opens before queuing, releasing the
reservation on rejection/failure or after registered-channel ownership takes
over.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/MysqlConnectProcessor.java:
##########
@@ -445,14 +445,26 @@ public void processOnce() throws IOException {
return;
}
- // dispatch
- dispatch();
- // finalize
- finalizeCommand();
-
- ctx.setCommand(MysqlCommand.COM_SLEEP);
- ctx.clear();
- executor = null;
+ if (!ctx.startMysqlCommand()) {
+ return;
+ }
+ // Account for planning and FE-local statements too: they may have no
Coordinator.
+ // Refuse new packets by closing the socket so a connection pool can
reconnect.
+ if (!QueryAdmissionController.INSTANCE.tryAcquire()) {
+ ctx.setKilled();
+ ctx.finishMysqlCommand();
+ channel.close();
Review Comment:
[P2] Keep connection teardown in the drain count
If an existing transaction receives a packet just after admission closes,
this branch sets the session killed and closes its channel without a
request/continuation reservation. The close listener runs
`unregisterConnection() -> closeTxn()` and can still be waiting for rollback.
Meanwhile `closeIdleMysqlConnection()` skips killed sessions, so the idle scan
can finish and `waitForGracefulShutdown()` can see zero work and stop FE
services before the rollback finishes, even with budget remaining. Admitted
`COM_QUIT` has the same gap because its count is released before `ReadListener`
calls cleanup. Track the complete close/rollback/temporary-table cleanup
lifetime, including refused packets, before publishing it as drained.
--
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]
