implement a connection pool

[c-cube]

see #44 for the use case. It also makes benchmark faster (e.g. a parallel merge sort of a list of 3000 elements on top of redis, which stresses latency a lot, goes from 15.2s to 11.2s using a pool of 32 connections on the local network interface).

To run the merge sort bench:

$ dune exec -- examples/examples.exe --name bench_merge_sort -n 3000

[thangngoc89]

Any updates on this PR?

[c-cube]

@thangngoc89 I think I'd be more comfortable if someone else reviewed my code. If @whilefalse has idea on how to actually make the with_pool test stuff work that'd be awesome (the ports don't match right now).

[thangngoc89]

@c-cube I see. I will blindly use this branch and hope for the best 🤞

[whilefalse]

@thangngoc89 I think I'd be more comfortable if someone else reviewed my code. If @whilefalse has idea on how to actually make the with_pool test stuff work that'd be awesome (the ports don't match right now).

Hey. I can certainly have a look, I’m not really an OCaml expert but if it’s related to my changes I’ll happily have a look.

Do you have any more information on what the problem with the ports is? I’ll try to have a look tomorrow when I’m back at my computer but any info you have would help.

[c-cube]

@whilefalse awesome! take your time, it's just that I rebased the thing and had some tricky merges for the test suite. My question is more: can we make the with_pool stuff work in your new shiny docker-compose setup. I think it's just that that is missing.

[Et7f3]

This was part of draft PR (i don't think you want to merge this)

[Et7f3]

The build issue are unrelated to windows but more to setup-ocaml. On my repo setup-ocaml doesn't manage to setup certificate, in this PR it can't restore cache. I tried to build with esy (it sandbox more than opam) and it builded. I try dune runtest and got:

�[0;1mFile "examples/dune", line 2, characters 20-30:�[0m
2 |  (libraries threads containers redis-lwt redis-sync)
                        ^^^^^^^^^^
�[1;31mError�[0m: Library "containers" not found.
Hint: try:
  dune external-lib-deps --missing @runtest
�[1mFile "src/client.ml", line 57, characters 20-38�[0m:
57 |       let compare = Pervasives.compare
                         �[1;35m^^^^^^^^^^^^^^^^^^�[0m
�[1;35mAlert�[0m deprecated: module Stdlib.Pervasives
Use Stdlib instead.

If you need to stay compatible with OCaml < 4.07, you can use the
stdlib-shims library: https://github.com/ocaml/stdlib-shims
�[1mFile "src/client.ml", line 542, characters 2-59�[0m:
542 |   val return_expected_status : string -> reply -> unit IO.t
        �[1;35m^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^�[0m
�[1;35mWarning�[0m 32 [unused-value-declaration]: unused value return_expected_status.
�[1mFile "src/client.ml", line 547, characters 2-42�[0m:
547 |   val return_int64 : reply -> Int64.t IO.t
        �[1;35m^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^�[0m
�[1;35mWarning�[0m 32 [unused-value-declaration]: unused value return_int64.
File "test/dune", line 6, characters 34-40:
6 |  (libraries containers unix redis ounit2))
                                      ^^^^^^
Error: Library "ounit2" not found.

Seem like missing deps. esy pulled OCaml 4.13

[c-cube]

I guess the opam files need:

depends: [
  …
 "containers" {with-test}
 "ounit2" {with-test}
]

[thomasmoawad]

Seems like there was some action on this PR as of Jan 7, 2022. Are there any updates? Has this task been accomplished by another PR?

[c-cube]

Not particularly, it's just that right now it needs some actual testing. If you're interested in the feature you could use opam pin to try it in your project for a little while (not in prod) and report here :)