mirage / ocaml-dns

OCaml implementation of the DNS protocol
BSD 2-Clause "Simplified" License
105 stars 43 forks source link

Timeout handling #277

Closed orbitz closed 2 years ago

orbitz commented 2 years ago

I am wondering if an improvement to the interface for dns-client would be something like a with_timeout function that the whole query is executed inside of. Something like:

val with_timeout : context -> 'a io -> 'a io

Rather than checking in each operation, it would be the whole connect, query, response.

My motivation is a little selfish as well in that I use my own concurrency monad and one of the features it provides is the ability to abort operations, but in order to cleanup resources safely, the operation that might be aborted needs to wrap itself in a cleanup operation (my API provides the equivalent of a finally operator). Currently the dns-client interface has no mechanism for this, but wrapping the whole query and response handling in a single operation would allow me to accomplish that.

My needs might be too niche to me, but if they fit into the needs of dns-client it'd be great to unify them. Otherwise I can fork and adjust the interface myself.

hannesm commented 2 years ago

interesting observtion -- so I went ahead, and implemented https://github.com/mirage/ocaml-dns/pull/278/commits/ae28897e4703bf387dba91a6b81cdc695f445559 -- does this suit your needs?

The API used by Dns_client, to be implemented by a Transport) still consists of:

I don't think we can further simplify it without dropping valid use cases (one separate connection per DNS request), although lwt and MirageOS implementations ignore close (and multiplex multiple queries over the same TCP connection).

orbitz commented 2 years ago

After thinking about it some more, I think for a finally-like construct to work like my monad implements it would need to be like:


val create_context : t -> context

val with_finally : context -> ~finally:(unit -> unit io) -> ('a, error) result io -> ('a, error) result io

val connect : context -> (unit, error) result io

val send : ...

val recv :...

val close : ...

And then Dns_client.Make would use that like:

let context = Transport.make_context t in
Transport.with_finally context ~finally:(fun () -> Transport.close context) (connect >>= send >>= recv)

For my monad, this would ensure that if it's aborted from the outside, everything is cleaned up properly, and if a timeout happens from within it would also work. I think this would also work with existing Lwt implementation (although I'm not sure how Lwt handles cancellation).

For a little context on why my monad is like this, among other things it allows me to write libraries that don't have to care about timeouts or other particular cancellation needs, it just needs to know how to cleanup in the case of a cancellation, so then I implement timeouts like:


Combinators.with_timeout ~timeout:(sleep 5.0) (Dns.gethostbyname ....)
>>= function
| `Ok res -> ..
| `Timeout -> ..

So that means if Dns_client can provide an interface that ensures resources can be cleaned up on an outside cancel, it will fit nicely in my API.

And again, I understand if semantics around my personal concurrency monad are not worth this client adhering to, it's pretty esoteric. But if it does that'd be great!

Thank you for the fast response.

hannesm commented 2 years ago

Thanks for your explanation. i think in your monad, you can do with the 6.1.0 Dns_client interface by only allocating the context in connect, and establish the connection, send and receive data in send_recv (under the timeout), and cleanup in close. The latter will always be called (regardless of whether send_recv errored out with a timeout or any other failure).

orbitz commented 2 years ago

Yes, that will work. Is connect and close even needed with this interface? One strength I just realized about doing it the way you suggest is I can cycle through nameservers if one is down and resend the query. That was possible in the previous interface as well, this interface just made it clearer to me that it's possible.

Again, thank you for the fast responses!

hannesm commented 2 years ago

What the lwt and mirage versions of the client are doing is in connect -- if there is no active TCP connection - to cycle through the available nameservers (using [happy-eyeballs[(https://gtihub.com/roburio/happy-eyeballs)).

In Mirage and Lwt - since they are TCP (and TLS) only (no UDP), they multiplex queries over a single connection. There close does nothing, and connect only if there is no existing TCP connection.