-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eio port #256
Eio port #256
Conversation
9c74a31
to
6a53d2d
Compare
Can we keep the LWT version available for OCaml 4.14? We will need to move services to OCaml 5 and EIO over a longer period of time, both for platform support reasons and just because it will take some time to port things over. |
The existing release will continue to work there. capnp-rpc is pretty stable, so I'm not expecting any major new features that would need back-porting. |
We'll take a release of this when it's ready. clarke and solver-service could use it. |
unix/network.ml
Outdated
Eio_unix.Resource.fd_opt flow | ||
|> Option.iter (fun fd -> | ||
Eio_unix.Fd.use_exn "TCP_NODELAY" fd @@ fun fd -> | ||
Unix.setsockopt fd Unix.TCP_NODELAY true |
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.
We might need to wrap this to catch
Unix.Unix_error(Unix.EOPNOTSUPP, "setsockopt", "")
Stumbled across this new change when using this branch on macOS with a unix domain socket.
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.
Sounds good. Not sure whether to do it here, or get ocaml-multicore/eio#575 finished. We could then do something like:
Eio.Net.connect ~sw net eio_addr ~options:[Try (Delay false)]
capnp-rpc-net/endpoint.ml
Outdated
let pp_error f = function | ||
| `Closed -> Fmt.string f "Connection closed" | ||
| `Msg m -> Fmt.string f m | ||
Eio.Flow.shutdown t.flow `All |
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.
Wrapping this in a try stops us crashing when a flow has already been closed. I.e.
try
Eio.Flow.shutdown t.flow `All
with | Eio.Io (Eio.Net.E Connection_reset _, _) as ex ->
Log.info (fun f -> f "%a" Eio.Exn.pp ex);
20a18fb
to
d0a8977
Compare
9955868
to
fd05557
Compare
b7aa08d
to
1029a53
Compare
30339fb
to
6e3148f
Compare
Should probably buffer up messages instead, but this solves the immediate problem.
This switches capnp-rpc from Lwt to Eio. One particularly nice side effect of this is that
Service.return_lwt
has gone, as there is no distinction now between concurrent and non-concurrent service methods.Notes:
This commit aims to keep the diff (relatively) small. Several more things are needed after this is merged. In particular: