Repository navigation
Conversation
vaintroub
force-pushed
the
main-MDEV-38741
branch
2 times, most recently
from
October 30, 2026 02:49
c31cfc0 to
70e9cd4
Compare
vaintroub
force-pushed
the
main-MDEV-38741
branch
from
November 2, 2026 12:37
3423790 to
cbeff4c
Compare
vaintroub
force-pushed
the
main-MDEV-38741
branch
from
November 3, 2026 01:36
cbeff4c to
b24c60e
Compare
The server notices that a client is gone only when it reads from or writes to the socket. While a statement runs, for example waiting for an InnoDB row lock, a disconnect goes unnoticed. The statement keeps running, and its transaction keeps owning its locks, so other connections wait for locks of a client that is not there. Watch the client socket from the end of login until the connection ends, or the client sends COM_QUIT, using only the events the OS reports on its own. When the client has gone, abort the running statement with the new KILL_DISCONNECT state, found by thread id like KILL does, through a kill_thd_locked() that KILL shares. A quitting connection ignores it. KILL_DISCONNECT is statement-level, like KILL QUERY. When it does abort a statement, the connection ends after the error of that statement is sent, so the commands that the client pipelined behind it do not run. A disconnect noticed while the connection is idle, before the statement starts, is remembered and applied when the next statement starts. The reason, "Client disconnected", is logged once, and a failed write of the error is not reported. An orderly COM_QUIT, or a close without it, is handled as before. The global variable disconnect_watcher, ON by default, can be set OFF to switch the watcher off. It affects the connections made after the change, and exists only where there is a watcher: Linux, Windows and the BSD systems, not the embedded server. If the watcher cannot start, the variable is OFF and cannot be set ON. Linux: an epoll set, polled by the connection accepting loop. EPOLLRDHUP is requested, as a close and a crashed client can be only a FIN. Windows sockets: WSAEventSelect(FD_CLOSE), with a wait per socket on a thread pool of our own that has a single thread, so the callbacks run one by one. Windows named pipes: the wait is for the exit of the local client process. A client that only closes the pipe is not noticed. BSD and macOS: kqueue, EV_EOF. Replication dump threads are watched too. mysqltest: dirty_close takes an optional "linger" (the default, a FIN) or "nolinger" (SO_LINGER 0, so the server gets a reset) argument. main.disconnect_lock_wait, disconnect_lock_owner (the client's transaction is rolled back and its locks are released), disconnect_wait_kinds (a CPU-bound statement and a table level lock wait, which do not poll the connection themselves), disconnect_lock_wait_idle (debug), disconnect_lock_wait_pipe (Windows) and sys_vars.disconnect_watcher_basic are new. mysql_client_test gets test_disconnect_pipelined: a client sends two commands in one write and half-closes while the first waits for a lock. The connection ends, and the second command is not run. disconnect_lock_wait and disconnect_lock_wait_pipe run with the watcher ON and OFF, the others need it ON. innodb.xa_recovery no longer needs the shortened lock wait timeout with the watcher ON. rpl_ssl1 and rpl_password_boundaries wait for the session of the slave to end before DROP USER, max_binlog_total_size and rpl_fail_register kill the dump thread only if it is still there, and rpl_get_lock suppresses the read error of a reset connection on Windows (errno 10054). Assisted-By: Claude:claude-sonnet-5-5
dispatch_command() now takes THD::LOCK_thd_data around set_command() and the check of the disconnect state, which makes the disconnect watcher and the start of a statement agree. The test lists every instrumented mutex wait of a statement, so each of its five statements has one more wait/synch/mutex/sql/THD::LOCK_thd_data in the "starting" stage, and the event ids after it are shifted by one. Assisted-By: Claude:claude-sonnet-5-5
The debug thread of mariabackup that emulates a DDL sends several statements in one query, and closed its connection as soon as the first of them returned. The server used to run the rest, now the disconnect watcher aborts the ALTER of a client that is gone, and mariabackup.innodb_ddl_on_intermediate_table hangs, as the debug sync signals the main thread waits for do not come. The thread reads the results of all the statements before it closes the connection. mysql_next_result() of libmysqld/libmysql.c, which mariabackup uses, called methods->next_result. Only the methods of the embedded server have it, the client in the server (sql-common/client.c, built with MYSQL_SERVER) has a shorter table, and the call crashed. Outside of the embedded server it calls read_query_result, which is next_result of the client library. Assisted-By: Claude:claude-sonnet-5-5
The members of MYSQL_METHODS after on_close_free existed only if !MYSQL_SERVER or EMBEDDED_LIBRARY, so the client in the server (sql-common/client.c, built with MYSQL_SERVER) had a shorter table than libmysqld/libmysql.c, which mariabackup builds as a client. mysql_next_result() read methods->next_result beyond the end of that table, and crashed. Declare all the members always, and set all of them in client_methods and in local_methods, next_result of a client is read_query_result. Revert the check of the embedded server in mysql_next_result(), it is not needed now. Assisted-By: Claude:claude-sonnet-5-5
vaintroub
force-pushed
the
main-MDEV-38741
branch
from
November 3, 2026 11:57
c9f9505 to
ebb7892
Compare
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
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.
Abort a running statement when its client disconnects. The server notices a gone client only when it reads from or writes to the socket, so a statement that waits for something else, like an InnoDB row lock, keeps running, and its transaction keeps owning its locks, while other connections wait for them.
Design
COM_QUIT(the watch is dropped when it is parsed, as the client closes its socket right after it, which is not a failure), using only events the OS reports on its own. The globaldisconnect_watchervariable (dynamic, ON by default, affects new connections) switches it off. It exists only where there is a watcher: Linux, Windows and the BSDs, not in the embedded server. If the watcher failed to start, the variable is OFF and cannot be set ON. The watcher finds the connection by thread id, likeKILLdoes, and acts on theTHDthroughkill_thd_locked(), whichKILLuses too, minus the privilege check.KILL_DISCONNECT, likeKILL QUERY. An idle or quitting connection is not killed, soCOM_QUIT, and a plain close, behave as before, with the usual "Aborted connection" line.THD::disconnect_statekeeps what the watcher found out:NONE,GONE(the client left while the connection was idle, the statement that starts next is aborted at once),ABORTING(it left while a statement ran, and the kill was applied), andLOGGED. A statement that starts when the state isABORTING, because the kill aborted nothing, as the statement had finished, treats it asGONE. The state is written underLOCK_thd_killandLOCK_thd_data, and the statement start takesLOCK_thd_dataaroundset_command()and the check, so a disconnect is either seen there or finds the running statement. It is reset for aTHDthat is reused from the thread cache.ER_CONNECTION_KILLED), the reason "Client disconnected" is logged once, and the connection ends, so the commands that the client pipelined behind the statement do not run. A failed write of the error is not reported.wsrep_kill_thd(), likeKILL. Not tested.close_connections()only stops the watcher from accepting watches, the resources (the epoll or kqueue fd, the Windows thread pool) are released inclean_up(), when no connection can use them.Platforms
EPOLLRDHUPis requested for every socket, as a close and a crashed client can be only a FIN, which is also how a unix socket reports a half-closeWSAEventSelect(FD_CLOSE)and a wait per socket, on a thread pool of our own with a single thread, so the callbacks run one by one like the epoll events. Polling with the AFD driver was tried and rejected: a pending poll makesselect(), which the vio layer waits with, lose the data notificationGetNamedPipeClientProcessId(), the client is impersonated if the process cannot be opened, andRevertToSelf()is checked). Remote pipes are skippedEV_EOFandEV_ERROR. It passed on the aarch64-macos builder at an earlier revision of this PR, not tested locally and not at this revisionLimitations
ER_CONNECTION_KILLEDif it can still read), and its connection ended.SLEEP()and the metadata lock waits poll the connection themselves, every 5 and 1 seconds. They are not changed, and are aborted by the polling, not by the watcher.Tests
main.disconnect_lock_wait(new): kills a waiting client over the default connection, over TCP closed with a FIN, and with a reset, and checks that the reason is logged once.main.disconnect_lock_owner(new): the transaction of the client is rolled back and its locks are released.main.disconnect_wait_kinds(new): a CPU-bound statement and a table level lock wait, which do not poll the connection themselves, and an idle client.main.disconnect_lock_wait_idle(new, debug): the client leaves after its statement was read and before it starts.main.disconnect_lock_wait_pipe(new, Windows): a real client is started on the pipe, waits for a lock, and is killed.test_disconnect_pipelinedinmysql_client_test(new): the client writes two commands to its socket in onesend(), half-closes, and the first waits for a lock. The connection ends, and the second command is not run. On Windows a reset can drop the error, soCR_SERVER_LOSTis accepted there.sys_vars.disconnect_watcher_basic(new). The tests that depend on the watcher run with it ON and OFF (include/disconnect_watcher.combinations), or need it ON (include/have_disconnect_watcher.inc); with it OFF the lock wait is not aborted, so the test releases the lock instead (rdiff files).mysqltest:dirty_close <connection> [linger|nolinger].nolingerresets the TCP connection,linger, the default, is what it was.innodb.xa_recoveryno longer needs the shortened lock wait timeout.rpl_ssl1andrpl_password_boundarieswait for the session of the slave to end (include/wait_until_user_sessions_end.inc),max_binlog_total_sizeandrpl_fail_registerkill the dump thread only if it is still there, as it may have ended already, andrpl_get_locksuppresses the read error of a reset connection on Windows (errno 10054).The full
mainsuite was run on Windows and Linux (Debug) at an earlier revision of this PR. At this revision the new and changed tests,kill,kill-2,kill_debug,shutdown,connect,sslandmysql_client_testpassed on Linux (Debug), and the new tests, withtest_disconnect_pipelined2000 times, passed on Windows. The only failures in the full runs are in my environment: large pages on Windows, and on WSL a world-writable config file and a missing git revision. Not tested: Galera, BSD and macOS.Cost
Connect and disconnect churn with libmariadb, 16 threads, 2000 connections per run, against the commit this PR is based on: no difference that is distinguishable from the noise (4 runs per cell). Plain TCP and AF_UNIX on Linux, and plain TCP on Windows, are within about 5%. With TLS, the handshake dominates (about 2000 conn/s on both systems) and the difference is within 2%. The per statement state check is not measurable either: about 216000 against 217000 statements per second, and about 52300 against 53600 connections per second, on a release build on Linux. A watcher event still fires for about 12% of the connections on Linux and about a third on Windows, when the FIN arrives before the server has parsed
COM_QUIT. Waiting for the server's EOF before closing the socket in the client removes all of them, but it is not part of this PR.If the VIO refactoring in #5584 lands first,
vio_transport_type()andvio_handle()replace the directhPipeaccess.🤖 Generated with Claude Code