n0-computer / iroh

peer-2-peer that just works
https://iroh.computer
Apache License 2.0
2.6k stars 164 forks source link

Improve MagicSock poll_ready behaviour #2676

Open flub opened 2 months ago

flub commented 2 months ago

Currently the iroh_net::magicsock::IoPoller will return Ready as soon as any of the channels (udp-ipv4, upd-ipv6 or relay) could send. This very easily results in Ready to be returned to eagerly: e.g. in a 1-1 transfer which is using udp-ipv4 only it would always return ready for the relay and udp-ipv6 thus never back off the poller.

We should improve the poller so that it does do this correctly. This may need more pollers as described in https://github.com/n0-computer/iroh/blob/f28bd27b87ee3e419835dce1fdeacbd0d895098d/iroh-net/src/magicsock.rs#L432-L445

Labelling as both a bug and a feature, because I don't think this behaviour is new since we use Quinn@0.11 but it still is not great.

matheus23 commented 2 months ago

I think improving this is important for correct backpressure. FWIW, in my testing I never actually hit any case where any UdpPoller returned Pending. That said, I used the iroh-net/bench, which runs localhost. It's entirely possible that localhost sending is just never slower than the encryption parts of QUIC, so essentially on localhost the socket can probably easily outrun the encryption.

The only other idea I have is testing this across machines, hoping this would make the socket "slower". But that's a little more involved. If anyone has other ideas, please LMK.