Add metrics about the solver-service behaviour

[moyodiallo]

No description provided.

[tmcgilchrist]

@moyodiallo Could you update this to the EIO version of the solver?

[moyodiallo]

The metrics is adapted to EIO version #71. It needs some tests with Grafana graphics.

cc: @mtelvers

[tmcgilchrist]

Everything else looks good. @moyodiallo co-ordinate with @mtelvers to deploy it to a solver worker and check the statistics are exported correctly. Do we still have a test solver pool available?

[tmcgilchrist]

This pattern (incr, handle, decr) feels unsafe in the presence of exceptions from handle request cc @talex5

[moyodiallo]

handle request needs to catch those exceptions otherwise we're losing a worker and that will result as a bug.

[talex5]

Yes, if a worker crashes then the whole service exits, so it's not strictly necessary to handle exceptions here, although it would make the code more robust to future changes. However, I don't think you need this counter. The number of running workers is always min n_workers n_jobs, so you can just calculate it as needed.

[moyodiallo]

Yes you're right, I did not get this min n_workers n_jobs. Thanks.

[moyodiallo]

Oh, we don't have n_jobs in the pool. All we have is waiting jobs (jobs Eio.Stream.t). It won't be precise if consider waiting jobs as n_jobs, ex. 2 jobs waiting and all the 8 workers busy.

[tmcgilchrist]

What would be useful to see is:

1. n_workers - static capacity of this solver_worker
2. queued_requests - requests waiting for a Fiber to run on
3. running_workers - Fibers actively running a solve job

That would let us answer questions like total solver_worker capacity available, utilisation of the total capacity, and saturation of the total capacity and per solver_worker saturation.

The number of running workers is always min n_workers n_jobs, so you can just calculate it as needed.

I thought from reading the code and https://github.com/ocaml-multicore/eio?search=1#multicore-support, Pool pre_forked a number of OS threads (Domains?) which would wake up when work was added to the Eio.Stream? Is that accurate @talex5 ?

[talex5]

That's right - there is a fixed pool of workers and they will all be busy unless there just aren't any jobs in the queue.

The issue here is that run_worker is running in a worker domain, and so can't (currently) update Prometheus metrics itself (which I guess it why it's updating an atomic instead and waiting for the main domain to report that). But the main domain can work out how many workers are running just by knowing how many jobs are outstanding, so this isn't necessary.

[moyodiallo]

That's right there's a way to do that.