Notice a relay that went quiet, instead of signing nothing in silence - #51
Merged
Merged
Conversation
Each relay thread dials, subscribes, and then blocks in `receive` until
its relay says something. When a peer goes away without closing, that
thread waits forever.
For a client that is a stale feed. For a signer it is worse, because of
how it looks from outside: remote signing stops, the approval window never
opens, the client's request times out with no reason given, and this
daemon's own `/info` still says the relay is connected. Nothing anywhere
says what happened.
Nothing inside that thread can say it either. A thread waiting on a dead
peer is the last thing able to notice it is waiting. So one more thread
watches all of them: it sends the keepalive, and it is the one still able
to act when no answer comes.
30s of silence ping
90s of silence half-close the socket, so the owner's blocked receive
returns and it reconnects through its own path
Amethyst's numbers, from their survey of 122 relays: idle timeouts cluster
around 60, 120, 240, 300 and 600 seconds, and a ping only holds a
connection open reliably when its interval is at most about half the
shortest. Ninety is three missed answers. Answering the relay's pings is
not a substitute for sending our own: a relay's idle timer counts what it
RECEIVES, so the pong the library sends in reply to its ping does nothing.
A fourth state on `/info` and in the GUI, `quiet`: the socket is open,
nothing has come down it for a while, and a keepalive is out unanswered.
Not disconnected, because nothing has failed; not plainly connected
either, because the last evidence of that is a minute old.
Takes nostr v0.8.0 (from v0.6.0), which is where `ping`, `idleMs` and
`shutdown` live, and which serializes writes on a connection: a connection
being kept alive has two users on two threads, and over TLS half a record
from each is a session that cannot be decrypted again.
NOT a socket receive timeout, and the reason is written in the file.
`SO_RCVTIMEO` makes the read return EAGAIN, and this io model treats
EAGAIN as a programmer bug and panics. It was tried in this very daemon
for the approval server (#47): it compiled, passed every test, and
panicked on the first wedged connection, turning a stall into a crash.
Verified by running the daemon against real relays, because of that:
ping at 2s, death at 6s: 50 pings sent, 50 answered, none given up on.
ping disabled, death at 4s: 12 live TLS sockets half-closed from the
keeper while their threads sat blocked inside the library's serve loop.
No panic, no EAGAIN, and 14 reconnects, which is the threads unwinding
through their own error path as intended.
Then 140 seconds on the real numbers: 5 pings, 0 given up on, 0
spurious reconnects.
Eight tests on the decision, which is pure over two numbers and needs no
socket. Each checked by breaking it: a connection that has never spoken
must not read as infinitely idle, the deadline must stay a multiple of the
interval, a quiet relay must be pinged once per interval and not once per
look, a reused slot must not inherit the last one's ping clock, and an
out-of-range slot must be ignored rather than written.
Adding the file to the test aggregator is its own note in that block now.
Eight new tests kept the count at 19 and looked exactly like eight
passing ones.
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.
Each relay thread dials, subscribes, and then blocks in
receiveuntil its relay says something. When a peer goes away without closing, that thread waits forever.For a client that is a stale feed. For a signer it is worse, because of how it looks from outside: remote signing stops, the approval window never opens, the client's request times out with no reason given, and this daemon's own
/infostill says the relay is connected. Nothing anywhere says what happened.Nothing inside that thread can say it either. A thread waiting on a dead peer is the last thing able to notice it is waiting. So one more thread watches all of them: it sends the keepalive, and it is the one still able to act when no answer comes.
receivereturns and it reconnects through its own pathAmethyst's numbers, from their survey of 122 relays: idle timeouts cluster around 60, 120, 240, 300 and 600 seconds, and a ping only holds a connection open reliably when its interval is at most about half the shortest. Ninety is three missed answers.
Answering the relay's pings is not a substitute for sending our own. A relay's idle timer counts what it RECEIVES, so the pong the library sends in reply to its ping does nothing. There is a test named after that.
A fourth state
quiet, on/infoand in the GUI: the socket is open, nothing has come down it for a while, and a keepalive is out unanswered. Not disconnected, because nothing has failed; not plainly connected either, because the last evidence of that is a minute old. Amber,warning.Not a receive timeout
SO_RCVTIMEOmakes the read return EAGAIN, and this io model treats EAGAIN as a programmer bug and panics. It was tried in this very daemon for the approval server (#47): it compiled, passed every test, and panicked on the first wedged connection, turning a stall into a crash. That is written at the top of the new file so the next person does not rediscover it.Verified by running the daemon
Against real relays, because of the above.
Tests
Eight, on the decision, which is pure over two numbers and needs no socket, thread or clock. Each checked by breaking it:
One thing worth knowing
Adding the file to the test aggregator in
main.zigis now a comment in that block. The eight new tests left the count at 19 and looked exactly like eight passing ones.Dependency
nostr v0.6.0 to v0.8.0, where
ping,idleMsandshutdownlive, and which serializes writes on a connection: a connection being kept alive has two users on two threads, and over TLS half a record from each is a session that cannot be decrypted again.