fix: re-arm process timeout on each retry - #5
Open
raprav wants to merge 2 commits into
Open
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
The process-level
timeoutis armed once by runnerty core, right before the first execution (lib/classes/process.js), and cleared at the top ofExecutor.end(). The retry branch ofend()re-runs the executor viaexecMain()without re-arming it, so from the first retry onwards the process runs completely unbounded. If a retry hangs (e.g. a connection that never settles), the process promise never resolves, the chain staysrunningforever and the scheduler silently skips every subsequent execution.The failure is also invisible by default: with
notificate_only_last_failthe intermediate error emits nothing,on_retryis rarely configured, and a retry does not logPROCESS STARTagain — a hung retry produces zero output.This is not theoretical. It is the mechanism behind three production outages in the Páez chains (Jun 30, Jul 1 and Aug 2): processes configured with
retries: 3andtimeout: { delay: '30s', action: 'error' }died with no trace at all. A forensic sweep over 53 days of logs shows the process timeout firing exactly once in production — always on a first attempt, never on the stalled ones, consistent with the stalls being silent retries.Fix
Re-arm the timer inside the retry branch, right before
execMain(), mirroring the arming code in runnerty core (killMain('timeout', { end: action })+process.time_out()). 12 lines, no new options, no behaviour change for processes withouttimeoutor withoutretries.Reproduction
test/executor-timeout-retry.test.jsdrives the realExecutorclass with anexec()that never settles (no database needed) and the exact process contract used by runnerty core:On
mainthe timeout fires once, the first retry runs unbounded and the process never settles (the test fails after its 3s watchdog). With this fix, every attempt (initial + retries) is bounded by the timeout and the process finally settles with an error.It can also be seen with a real chain: a
SELECT SLEEP(600)process withretries: 3andtimeout: { delay: '5s', action: 'error' }— onmainrunnerty hangs forever after the first retry; with the fix it logs 4 timeouts, exhausts the retries and releases the chain in ~35s.🇪🇸 Español
Problema
El
timeouta nivel de proceso lo arma runnerty core una sola vez, justo antes de la primera ejecución (lib/classes/process.js), yExecutor.end()lo limpia nada más entrar. La rama de retries deend()relanza el executor víaexecMain()sin re-armarlo: a partir del primer retry el proceso corre sin ningún límite. Si un retry se cuelga (p. ej. una conexión que nunca responde), la promesa del proceso no se resuelve jamás, la cadena se quedarunningpara siempre y el scheduler omite en silencio todas las ejecuciones siguientes.Además el fallo es invisible por defecto: con
notificate_only_last_failel error intermedio no emite nada,on_retryrara vez está configurado, y un retry no vuelve a loguearPROCESS START— un retry colgado produce cero output.No es teórico: es el mecanismo de tres paradas de producción en las cadenas de Páez (30-jun, 1-jul y 2-ago): procesos con
retries: 3ytimeout: { delay: '30s', action: 'error' }murieron sin dejar una sola línea. Un barrido forense de 53 días de logs muestra el timeout de proceso disparando exactamente una vez en producción — siempre en un primer intento, nunca en los cuelgues, consistente con que los cuelgues fueran retries silenciosos.Fix
Re-armar el timer dentro de la rama de retry, justo antes de
execMain(), espejando el código de armado de runnerty core (killMain('timeout', { end: action })+process.time_out()). 12 líneas, sin opciones nuevas, sin cambio de comportamiento para procesos sintimeouto sinretries.Reproducción
test/executor-timeout-retry.test.jsejercita la claseExecutorreal con unexec()que nunca termina (no necesita base de datos) y el contrato de proceso exacto que usa runnerty core:En
mainel timeout dispara una vez, el primer retry corre sin límite y el proceso no settlea nunca (el test falla tras su watchdog de 3s). Con el fix, cada intento (inicial + retries) queda acotado por el timeout y el proceso termina finalmente con error.También se ve con una cadena real: un proceso
SELECT SLEEP(600)conretries: 3ytimeout: { delay: '5s', action: 'error' }— enmainrunnerty se queda colgado para siempre tras el primer retry; con el fix loguea 4 timeouts, agota los retries y libera la cadena en ~35s.