an AbortSignal on any statement, wired to the engine's interrupt - #3
Merged
Merged
Conversation
Every statement takes a third argument now, and what is in it today is a signal. It is the one JavaScript already has, so the timeout a caller writes with AbortSignal.timeout, the signal a framework hands a request handler, and an AbortSignal.any composed out of both all work without anything being adapted. When it fires, the engine's interrupt is raised and the executor notices it at a boundary it was already stopping at, so a statement ends inside a vector of rows rather than at the end of the scan. The promise rejects with the signal's own reason, which is what fetch does, so a caller gets back their own error object rather than a description of it. Two things make this more than adding a listener. The interrupt belongs to the connection rather than to the statement, so a stop raised a moment too late would end the next statement instead of the one it was meant for: the statement puts the interrupt back down as it leaves, under a flag the listener reads before it raises anything. And a signal can fire before the statement reaches the thread it runs on, which is a stop nobody is there to hear: the listener and the statement set two words in the opposite order, so whichever is second sees what the other did and the statement is refused before the engine sees it. The listener is taken off the signal when the statement ends, whether it answered, failed or was stopped, because a request's signal outlives the statements run under it and a listener left on one is a leak that grows with the traffic. Watching a signal costs about two microseconds a statement here, against nine for the promise and the threadpool round trip that carry it.
33 tasks
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.
Every statement takes a third argument now, and what is in it today is a signal.
It is the signal JavaScript already has, so a timeout written like that, the signal a framework hands a request handler, and an
AbortSignal.any([...])composed out of both all work here without anything being adapted. When it fires, the engine's interrupt is raised, the executor notices it at a boundary it was already stopping at, and the statement ends inside a vector of rows rather than at the end of the scan. Measured on the statement the tests use, an abort comes back three to eight milliseconds after it was asked for, against six hundred for the same statement run to the end.What the promise rejects with is the signal's own reason, which is what
fetchdoes:AbortSignal.timeout(50)rejects with the runtime'sTimeoutError,controller.abort(new RequestGone())rejects with theRequestGonethe caller made, and a barecontroller.abort()rejects with the runtime'sAbortError. A caller who already wrotecatcharound a timeout gets their own object back rather than a GQLSTATUS to translate.The two races
The interrupt belongs to the connection rather than to the statement, since the session arms one handle when it is built and nothing in the engine ever clears it. A stop raised a moment after a statement finished would therefore end the next statement on that connection. So the statement puts the interrupt back down as it leaves, under a flag the listener reads before it raises anything, and a test asserts the statement after an aborted one answers normally.
A signal can also fire before the statement reaches the thread it runs on, which is a stop nobody is there to hear. The listener and the statement share two words and set them in the opposite order, so whichever of them is second sees what the other did: a statement that finds the signal already fired is refused before the engine sees it at all.
The listener
It is taken off the signal when the statement ends, whether it answered, failed or was stopped. A request's signal outlives the statements run under it, often by a whole request, so a listener left on one is a leak that grows with the traffic rather than with the code. A test runs sixteen statements against one signal and asserts the signal has no listeners left on it afterwards, through
events.getEventListeners.Cost
Watching a signal costs about two microseconds a statement, against nine for the promise and the threadpool round trip that carry the cheapest statement there is, measured by alternating the two shapes across rounds.
npm run benchgained a row for it, beside the same statement without one.What is here
src/cancel.rsis the watch: the listener, the two words, the reason, and the release.src/conn.rsreadsoptions.signalon the thread that owns the runtime, refuses anything that is not anAbortSignalas aZuUsageError, and rejects an aborted statement with the signal's reason.test/abort.test.mjsis eight tests, and it calibrates rather than hardcoding a duration: it times the statement once with nobody stopping it, then asks for a stop a tenth of the way through and asserts the answer came back in less than half the time. A debug build on a busy laptop and a release build on a quiet machine are two orders of magnitude apart, and a number chosen for one of them is a test that fails on the other.One thing worth writing down for the next person: napi passes a bare Rust tuple to a JavaScript function as a single argument.
addEventListenercalled that way complains that its arguments were not specified while three of them sit in the call.FnArgsis what spreads a tuple into arguments.Milestone: DX3, tamnd/zu#169, the
AbortSignalpart of theawait using/AsyncIterable/ Web Streams /AbortSignalline.