-
Notifications
You must be signed in to change notification settings - Fork 28
Conversation
Currently, I define the default ConnectTimeout here. I could put this in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, also, with this timeout this pr does all the job of libp2p/go-libp2p-swarm#70
@@ -13,6 +14,10 @@ import ( | |||
mafmt "github.com/whyrusleeping/mafmt" | |||
) | |||
|
|||
// DefaultConnectTimeout is the (default) maximum amount of time the TCP | |||
// transport will spend on the initial TCP connect before giving up. | |||
var DefaultConnectTimeout = 5 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, while I think this should be enough, it would be great to have some data to show what percentage of successful dials would not be covered by this timeout. (I mean, 5s is enough to finish 3way handshake earth<->moon with about 1.1s to spare so..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this depends on a lot of things. To ping a random machine at MIT (CSAIL), it takes me (from the bay area) anywhere form 90ms to 1.1s:
PING daphne.csail.mit.edu (128.52.129.29) 56(84) bytes of data.
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=1 ttl=47 time=92.4 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=2 ttl=47 time=165 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=3 ttl=47 time=90.2 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=4 ttl=47 time=1088 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=5 ttl=47 time=90.2 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=6 ttl=47 time=110 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=8 ttl=47 time=84.8 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=7 ttl=47 time=1095 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=9 ttl=47 time=136 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=10 ttl=47 time=259 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=11 ttl=47 time=1001 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=13 ttl=47 time=90.7 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=12 ttl=47 time=1100 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=14 ttl=47 time=140 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=15 ttl=47 time=85.0 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=17 ttl=47 time=101 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=18 ttl=47 time=1111 ms
64 bytes from daphne.csail.mit.edu (128.52.129.29): icmp_seq=19 ttl=47 time=295 ms
Now, one issue here is the SYN retransmission timeout. On Linux and Windows, the first retransmission attempt is after 3s so 5s will give you approximately two tries. On MacOS, the timeout is 6s, so you'll only get one attempt.
We'll probably want to tweak this but 5s is good enough for now. |
No description provided.