swarm: delay /webrtc-direct dials by 1 second #3078
Conversation
Previously these addresses weren't delayed at all. When I initially did the ranker implementation, I was too conservative regarding what to delay. So, non QUIC, WebTransport, TCP, WS addresses were ignored in ranking. We only ever need to dial /webrtc-direct when there's no other address available for the peer, in which case we will dial the addresses immediately. This would have helped with libp2p/js-libp2p#2805 as there would have been fewer peers dialing webrtc and then cancelling because they connected on a better transport. This also introduces an additional 1 second delay for any fancy non IP transports
sukunrt
force-pushed
the
sukun/webrtc-delay
branch
from
9796b8d to
0df12d6
Compare
sukunrt
force-pushed
the
sukun/webrtc-delay
branch
from
0df12d6 to
b352a54
Compare
| @@ -199,3 +203,72 @@ func TestAddrFactorCertHashAppend(t *testing.T) { | |||
| return hasWebRTC && hasWebTransport | |||
| }, 5*time.Second, 100*time.Millisecond) | |||
| } | |||
|
|
|||
| func TestWebRTCDirectDialDelay(t *testing.T) { | |||
There was a problem hiding this comment.
I don't understand this test. Is it trying to make sure that if many peers are connected to a server (via QUIC?), that another peer can also connect to that server via WebRTC?
There was a problem hiding this comment.
There are two tests.
- TestOnlyWebRTCDirectDialNoDelay: Tests that dials to a peer with only webrtc address aren't delayed.
- TestWebRTCWithManyQUICConnections: Tests that in presence of quic addresses webrtc dials are actually delayed. In case there's no happy eyeballs, the 200 initial dials would have taken up all the goroutines available for handshakes in the webrtc listener.
There was a problem hiding this comment.
In case there's no happy eyeballs, the 200 initial dials would have taken up all the goroutines available for handshakes in the webrtc listener.
But you wait for the Connect to finish, so wouldn't the handshake be complete at this point?
There was a problem hiding this comment.
Yes, the QUIC handshake will complete not the webrtc one. The webrtc handshake will be cancelled. As there's no way to signal this cancellation in webrtc, the server will have to wait for timeout on this handshake. So, at this point if we get a new webrtc handshake attempt, it'll fail because there were no free listener goroutines. This all happens when there's no happy eyeballs.
With happy eyeballs, the quic handshake should complete without any webrtc dials.
sukunrt
force-pushed
the
sukun/webrtc-delay
branch
18 times, most recently
from
4114296 to
a28ecec
Compare
sukunrt
force-pushed
the
sukun/webrtc-delay
branch
from
a28ecec to
e675e4b
Compare
Previously these addresses weren't delayed at all. When I initially did the ranker implementation, I was too conservative regarding what to delay. So, non QUIC, WebTransport, TCP, WS addresses were ignored in ranking. We only ever need to dial /webrtc-direct when there's no other address available for the peer, in which case we will dial the addresses immediately.
This would have helped with libp2p/js-libp2p#2805 as there would have been fewer peers dialing webrtc and then cancelling because they connected on a better transport.
This also introduces an additional 1 second delay for any fancy non IP transports