feat(iroh-relay): send regular pings to check the connection #3113
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3113/docs/iroh/ Last updated: 2025-01-13T16:17:30Z |
iroh/src/magicsock/relay_actor.rs
Outdated
| @@ -623,13 +641,19 @@ impl ActiveRelayActor { | |||
| state: &mut ConnectedRelayState, | |||
| client_stream: &mut iroh_relay::client::ClientStream, | |||
| ) -> Result<()> { | |||
| const SEND_TIMEOUT: Duration = Duration::from_secs(10); // TODO: what should this be? | |||
There was a problem hiding this comment.
what should this be?
There was a problem hiding this comment.
contraints are:
- MAX_PACKET_SIZE is set to 64 KiB
- The client sends up to 20 packets at once (if this is too annoying we could take out that batching)
- so worst-case a single send is 200 KiB
- Practically I don't think 64 KiB packets will be sent
- 1500 bytes/packet is more likely
- so ~30 KiB is a more typical max send size
I'm not sure what lower limit of bandwidth we should assume here.
There was a problem hiding this comment.
set to PING_INTERVAL for now, as discussed
iroh/src/magicsock/relay_actor.rs
Outdated
| @@ -623,13 +641,19 @@ impl ActiveRelayActor { | |||
| state: &mut ConnectedRelayState, | |||
| client_stream: &mut iroh_relay::client::ClientStream, | |||
| ) -> Result<()> { | |||
| const SEND_TIMEOUT: Duration = Duration::from_secs(10); // TODO: what should this be? | |||
There was a problem hiding this comment.
contraints are:
- MAX_PACKET_SIZE is set to 64 KiB
- The client sends up to 20 packets at once (if this is too annoying we could take out that batching)
- so worst-case a single send is 200 KiB
- Practically I don't think 64 KiB packets will be sent
- 1500 bytes/packet is more likely
- so ~30 KiB is a more typical max send size
I'm not sure what lower limit of bandwidth we should assume here.
|
One feature we haven't considered here yet is to defer sending the pings if there's real traffic. That is, every time you receive some data you can reset the ping interval and postpone sending a ping. This is approximately what QUIC does. |
Yes, I think we should do that 👍 this makes the overhead non existent during activity |
TCP does not guarantee that the connection is healty, so introduce protocol-level pinging to force us to reconnect if needed.
dignifiedquire
force-pushed
the
feat-relay-server-ping
branch
from
7307926 to
75c578a
Compare
|
added the resetting of the ping intervals when messages are received |
Description
Because of issues with TCP, we now send regular ping-pongs from client to server and vice versa.
Breaking Changes
Notes & open questions
Change checklist