hs-connection
hs-connection copied to clipboard
Asynchronous search for successful IP
This is to introduce a minimal (and not complete) implementation of RFC 6555 and RFC 8305.
Instead of doing manual asynchronous DNS lookup for AAAA and A records, it relies on getAddrInfo.
Available IP addresses are chosen concurrently.
The reason for the change is to avoid long timeouts when a domain has AAAA DNS record but no IPv6 connectivity, which is a relatively common misconfiguration.
I generally like the idea a lot. A few nitpicks:
- this keeps the bug described in #51
-
forConcurrently
will not cancel the other attempts, they may keep running in the background, should we usewithAnyCancel
? - even if we use
withAnyCancel
I'm not 100% convinced we won't end up with 2 or more open connections in some edge cases - or am I missing something? Think one action completes just as another one has opened the connection and is about totryPutMVar
. This may be tricky to get right.
Provided those are fixed I'd be in favor of merging this over #52.
I generally like the idea a lot. A few nitpicks:
* this keeps the bug described in [HostNotResolved can never happen? #51](https://github.com/vincenthz/hs-connection/issues/51)
I rebased the change on top of mulderr:master
.
* `forConcurrently` will not cancel the other attempts, they may keep running in the background, should we use `withAnyCancel`?
I expected forConcurrently
to throw AsyncCancelled to all the threads, am I not right about it? I also tried an implementation based on recursion with withAsync
so we only create as many asyncs as needed, but it makes code a little more complicated. I can push such a change, would make sense to try.
* even if we use `withAnyCancel` I'm not 100% convinced we won't end up with 2 or more open connections in some edge cases - or am I missing something? Think one action completes just as another one has opened the connection and is about to `tryPutMVar`. This may be tricky to get right.
Oh, that's a good point!
I pushed a commit where tryPutMVar
is under the same mask
called by bracketOnError, so the behavior should be atomic. But I am not sure now if it's problematic (can there be a deadlock?) to have a masked tryPutMVar
, hopefully not.
it's looking interesting, but honestly no time to review / look in details on what's happenning. anyone interested in taking over or just having writable permission to this repo ?
I would be happy to help or even take over if no one else comes forward.
--
I ran some simple tests. LGTM. Also I see http-client
is already using an almost identical implementation.
It would be ideal to have Connection Attempt Delay be configurable through ConnectionParams
but that would cause quite a lot of turbulence downstream so I'd put that off for a possible major version down the line.