search

sipcapture / heplify-server

HEP Capture Server for HOMER
https://sipcapture.org
GNU Affero General Public License v3.0
184 stars 85 forks source link

DBWorker limited by # of CPUs which is not always a good gauge of how to limit a DB Pool. #463

Open lwahlmeier opened 3 years ago

lwahlmeier commented 3 years ago

Currently the DBWorker is limited by the number of cores on the host machine:

    if worker > runtime.NumCPU() {
        worker = runtime.NumCPU()
    }

A DB Connection pool is not usually bound to the number of CPU cores as it a lot of cases it has more to do with network and DB latency.

It would be nice to either remove this limit all together, or implement a way of overriding it.

Let me know if either of these options will work and I will happily put in a PR for it.

negbie commented 3 years ago

Hi @lwahlmeier, hmm that code is quite old and not in use in our production for some time now since we moved to ClickhouseDB which needs a different concurrency pattern to work efficently. So take my words with care but I think you can just remove that condition if you keep in mind that this setting is for the worker count. Every worker will open/maintain

p.db.SetMaxOpenConns(config.Setting.DBWorker * 4)
p.db.SetMaxIdleConns(config.Setting.DBWorker)

So while it can improve performance on machines with high CPU core count I think removing this restriction will reduce the performance for some users who often thinks more is better and set this value to high which will cause contention. Nevertheless I'm ok with it and would kindly ask you for a PR.