Skip to content
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

Add --database-host option #93

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mattpatey
Copy link

Add --database-host option to define a non-local PostgreSQL database to be used by Critic host at installation time. By default --database-host is set to "localhost", which is how Critic is currently hardwired to run. This makes running Critic in a Docker container or other environment where the database host is not running on the web host feasible.

@jensl
Copy link
Owner

jensl commented Jul 27, 2015

Thanks for the patch! I've imported it to our Critic system for review, and mostly for automatic testing. (Which shows some errors: using "-h localhost" to psq is not quite equivalent to not using a -h argument at all, it seems.)

Critic review here: https://critic-review.org/r/341

One initial thought I have about this otherwise is that there might be some performance with using a separate database hose. I'd imagine some kind of connection caching or such might be necessary then, which I simply haven't bothered looking into since it definitely doesn't seem necessary when connecting to the database server locally.

@mattpatey
Copy link
Author

(Which shows some errors: using "-h localhost" to psq is not quite equivalent to not using a -h argument at all, it seems.)

I've set the default database host to /tmp, which is the documented default value.

One initial thought I have about this otherwise is that there might be some performance with using a separate database hose. I'd imagine some kind of connection caching or such might be necessary then, which I simply haven't bothered looking into since it definitely doesn't seem necessary when connecting to the database server locally.

How about using psycopg2's connection pooling feature?

@mattpatey
Copy link
Author

@jensl When trying to figure out what to set maxconn to I assumed that it would be sufficient to use the same value as in the WSGIDaemonProcess threads parameter (default: 25), however looking at the process tree on a system running Critic, there appeared to be 27 threads for each Apache process running Critic. Any idea where the two additional threads might come from? And what would you suggest setting the maximum number of connections to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants