fix: bound connection acquisition with queryTimeout to prevent handshake hangs#33
Draft
raprav wants to merge 1 commit into
Draft
fix: bound connection acquisition with queryTimeout to prevent handshake hangs#33raprav wants to merge 1 commit into
raprav wants to merge 1 commit into
Conversation
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.
🇬🇧 English
Problem
queryTimeout(#30) turned out to be necessary but not sufficient. In production (AWS RDS MySQL 8.0, Runnerty chains on cron) we hit two more permanent hangs with the 3.3.0 fix deployed and verified in the running image, and the timeout never fired.The reason is that a MySQL operation has two phases, and #30 only covers the second one:
connectTimeoutas soon as the first byte arrives from the server (lib/base/connection.js,stream.on('data')). If the server stalls halfway through the handshake — in our case it emitted an out-of-sequence auth packet ~12–24 s into the connection — the client logsWarning: got packets out of order. Expected 2 but received 1, keeps waiting for an auth response that never comes, andpool.getConnection()never calls back. No timer covers this window.timeoutinsideQuery.start(), which only runs once a fully-handshaken connection executes the command. If phase 1 never completes, this timer never arms.Observable evidence from 14 days of production logs: 2 occurrences of the
packets out of orderwarning, each one immediately followed by a permanent chain hang (until host restart), and zero occurrences ofPROTOCOL_SEQUENCE_TIMEOUT— the phase-2 timer never got the chance to arm.Fix
Reuse the same
queryTimeoutdeadline to bound phase 1: before dispatching the query,acquireConnection()proves the pool can deliver a healthy connection within the deadline (then releases it back to the per-execution pool, wherepool.query()reuses it — measured overhead: negligible). On expiry it destroys every connection held by the pool (a connection stuck mid-handshake is checked out, sopool.end()alone would never close it) and rejects, so the process always ends and the chain is never blocked.No new configuration:
queryTimeoutnow bounds each phase independently. Fully backwards compatible — withoutqueryTimeoutnothing changes.Reproduction & validation
test/acquire-timeout-repro.js(manual script, not wired tonpm test) reproduces the production failure byte-for-byte: real MySQL 8.0 greeting → server ignores the clientHandshakeResponse→ desyncedcaching_sha2full-auth packet (seq 1 while the client expects 2) → silence.queryTimeout(3003 ms)SELECT 1+queryTimeoutvs real MySQL 8.0SELECT SLEEP(60)+queryTimeout: 3000vs real MySQL 8.0SELECT 1withoutqueryTimeout🇪🇸 Español
Problema
El
queryTimeout(#30) resultó necesario pero no suficiente. En producción (AWS RDS MySQL 8.0, cadenas Runnerty bajo cron) sufrimos dos cuelgues permanentes más con el fix de la 3.3.0 desplegado y verificado en la imagen corriendo, y el timeout nunca disparó.La razón es que una operación MySQL tiene dos fases, y #30 solo cubre la segunda:
connectTimeouten cuanto llega el primer byte del servidor (lib/base/connection.js,stream.on('data')). Si el servidor se atasca a mitad de handshake — en nuestro caso emitió un paquete de auth fuera de secuencia a los ~12–24 s — el cliente logueaWarning: got packets out of order. Expected 2 but received 1, se queda esperando una respuesta de auth que nunca llega, ypool.getConnection()no retorna jamás. Ningún timer cubre esta ventana.timeoutde query dentro deQuery.start(), que solo corre cuando una conexión con handshake completo ejecuta el comando. Si la fase 1 no termina, este timer nunca se arma.Evidencia observable en 14 días de logs de producción: 2 apariciones del warning
packets out of order, cada una seguida inmediatamente de un cuelgue permanente de la cadena (hasta reiniciar el host), y cero apariciones dePROTOCOL_SEQUENCE_TIMEOUT— el timer de la fase 2 nunca llegó a armarse.Fix
Reutilizamos el mismo deadline de
queryTimeoutpara acotar la fase 1: antes de despachar la query,acquireConnection()comprueba que el pool puede entregar una conexión sana dentro del plazo (y la devuelve al pool por-ejecución, dondepool.query()la reutiliza — overhead medido: despreciable). Si expira, destruye todas las conexiones del pool (una conexión a medio handshake está checked-out, así quepool.end()por sí solo nunca la cerraría) y rechaza, de modo que el proceso siempre termina y la cadena nunca queda bloqueada.Sin configuración nueva:
queryTimeoutahora acota cada fase de forma independiente. Totalmente retrocompatible — sinqueryTimeoutno cambia nada.Reproducción y validación
test/acquire-timeout-repro.js(script manual, no cableado anpm test) reproduce el fallo de producción byte a byte: greeting real de MySQL 8.0 → el servidor ignora laHandshakeResponsedel cliente → paquete full-auth decaching_sha2desincronizado (seq 1 cuando el cliente espera 2) → silencio.queryTimeout(3003 ms)SELECT 1+queryTimeoutcontra MySQL 8.0 realSELECT SLEEP(60)+queryTimeout: 3000contra MySQL 8.0 realSELECT 1sinqueryTimeout