github-actions[bot] commented on code in PR #69421:
URL: https://github.com/apache/doris/pull/69421#discussion_r4373744664
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -779,6 +781,37 @@ public void setCommand(MysqlCommand command) {
this.command = command;
}
+ /** Reserve an idle MySQL connection before dispatching a newly received
packet. */
+ public synchronized boolean startMysqlCommand() {
+ if (isKilled) {
+ return false;
+ }
+ // COM_QUERY also covers the short interval before dispatch decodes
the command.
+ // Otherwise the shutdown thread could mistake a received packet for
an idle socket.
+ command = MysqlCommand.COM_QUERY;
+ mysqlCommandInProgress = true;
+ return true;
+ }
+
+ public synchronized void finishMysqlCommand() {
+ command = MysqlCommand.COM_SLEEP;
+ mysqlCommandInProgress = false;
+ }
+
+ /** Close an idle socket without cancelling an executor that has just
finished. */
+ public boolean closeIdleMysqlConnection() {
+ synchronized (this) {
+ if (getConnectType() != ConnectType.MYSQL || command !=
MysqlCommand.COM_SLEEP
+ || mysqlCommandInProgress || isKilled) {
+ return false;
+ }
+ // Paired with startMysqlCommand: a packet racing idle closure
must not run.
+ isKilled = true;
+ }
+ closeChannel();
Review Comment:
[P2] Finish temporary-table cleanup for idle MySQL sessions
For a sleeping MySQL session that owns a temporary table, this new close
path only closes the channel. AcceptListener's socket close listener calls
ConnectPoolMgr.unregisterConnection(), which aborts the transaction and removes
the context from the pool, but never calls cleanup()/deleteTempTable(). A quiet
local socket close invokes XNIO's [close
callback](https://github.com/xnio/xnio/blob/3.8.14.Final/api/src/main/java/org/xnio/conduits/ReadReadyHandler.java#L53-L67),
so a ReadListener task need not run the remaining cleanup. Once removed,
neither another idle pass nor a later COM_QUIT can recover this owner.
Temporary tables are journaled and the fallback TemporaryTableMgr is disabled,
leaving their metadata and tablets orphaned after the shutdown-initiated
disconnect, including sessions that would otherwise close normally during the
remaining grace wait. Perform the normal session cleanup on the bounded idle
worker, preserving its context binding and avoiding executor cancellat
ion.
##########
fe/fe-core/src/main/java/org/apache/doris/DorisFE.java:
##########
@@ -726,19 +733,76 @@ public static boolean isServerReady() {
}
private static void gracefulShutdown() {
- // wait for all queries to finish
+ // Health is already 503, but keep serving during LB / DNS convergence.
try {
- long now = System.currentTimeMillis();
- List<Coordinator> allCoordinators =
QeProcessorImpl.INSTANCE.getAllCoordinators();
- while (!allCoordinators.isEmpty() && System.currentTimeMillis() -
now < 300 * 1000L) {
- Thread.sleep(1000);
- allCoordinators =
QeProcessorImpl.INSTANCE.getAllCoordinators();
- LOG.info("waiting {} queries to finish before shutdown",
allCoordinators.size());
- }
- } catch (Throwable t) {
- LOG.error("", t);
+ LOG.info("graceful shutdown: serving during drain for {} seconds",
Config.graceful_shutdown_drain_seconds);
+ TimeUnit.SECONDS.sleep(Math.max(0,
Config.graceful_shutdown_drain_seconds));
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ LOG.warn("graceful shutdown drain interrupted", e);
}
+ // Close admission before the listener: an accepted connection can
race its closure.
+ QueryAdmissionController.INSTANCE.stopAccepting();
+ if (qeService != null) {
+ qeService.stopAcceptNewConnections();
+ }
+ try {
+ long deadline = System.nanoTime()
+ + TimeUnit.SECONDS.toNanos(Math.max(0,
Config.grace_shutdown_wait_seconds));
+ waitForGracefulShutdown(ExecuteEnv.getInstance().getScheduler(),
deadline);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ LOG.warn("graceful shutdown wait interrupted", e);
+ } catch (Throwable t) {
+ LOG.error("graceful shutdown failed while draining requests", t);
+ }
LOG.info("graceful shutdown finished");
}
+
+ static void waitForGracefulShutdown(ConnectScheduler scheduler, long
deadline) throws InterruptedException {
+ // XNIO invokes close listeners synchronously, including transaction
rollback RPCs.
+ // Keep those waits on a daemon worker so even an idle session cannot
stall the deadline.
+ ExecutorService idleCloser =
ThreadPoolManager.newDaemonFixedThreadPool(
+ 1, 1, "graceful-idle-mysql-closer", false);
+ try {
+ Future<?> idleCleanup = null;
+ boolean cleanupStartedIdle = false;
+ while (true) {
+ if (idleCleanup == null) {
+ cleanupStartedIdle =
QueryAdmissionController.INSTANCE.getActiveRequests() == 0;
+ idleCleanup = idleCloser.submit(() ->
scheduler.closeIdleMysqlConnections(deadline));
+ }
+ int activeRequests =
QueryAdmissionController.INSTANCE.getActiveRequests();
Review Comment:
[P2] Recheck request quiescence after collecting pending work
This count can become stale during the handoff from a cached Flight result
to CloseSession rollback. With a final idle scan already complete, shutdown can
read zero here while a transactional INSERT VALUES status ticket is still
cached. CloseSession then acquires its continuation, clears the cache through
releaseProtocolSession(), and waits in closeTxn() for the BE rollback RPC (up
to five seconds). CloseSession holds no command lock, so the subsequent
coordinator/result checks can both report empty and the loop exits using the
old zero despite the live reservation and remaining grace budget. [Arrow 19
FlightServer.close()](https://github.com/apache/arrow-java/blob/v19.0.0/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java#L135-L157)
then force-closes RPCs after three seconds, losing the accepted close
response. The earlier missing-reservation issue is fixed; this is a separate
consumer-side snapshot race in DorisFE. Revalidate the request count after the d
ependent predicates, or use an equivalent coherent quiescence check before
breaking.
--
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]
