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_MAX_CONNS config setting #1108

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Add DATABASE_MAX_CONNS config setting #1108

merged 1 commit into from
Oct 9, 2020

Conversation

rfwatson
Copy link
Contributor

Tiny change to allow the database connection pool size to be configured externally (useful for those of us deploying to databases with limited connection count allowances)

@rfwatson
Copy link
Contributor Author

Just spotted that this is a duplicate of #949 - feel free to close this one.

@PrivatePuffin
Copy link

This one is actually passing the checks and the other one isn't ;)

@whyvra
Copy link

whyvra commented Oct 6, 2020

Would love to see this PR merged in. When I tried to deploy bitwarden_rs today, it filled up all of my pgpool connections. I find it kinda crazy that it needs 10 simultaneous connections to the database.

+1

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 6, 2020

this PR has some conflicts so a simple merge is not possible.

@rfwatson
Copy link
Contributor Author

rfwatson commented Oct 6, 2020

I’ll fix the conflicts later if I have time

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 8, 2020

Well, there arn't any conflicts it seems.

But i do have one remark. It would be best if there is some validation done on the input value.
r2d2 states that if the value is 0 it will panic. I would like to prevent that from happening at all.

So i think at least a check that is is 1 or higher, maybe even a max, because i wouldn't want to have 1000 connections to my database by accident.

@rfwatson
Copy link
Contributor Author

rfwatson commented Oct 9, 2020

Updated in b9daa59

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 9, 2020

LGTM, and since there seems to be quite some request about this i think it is fine by me.

@dani-garcia dani-garcia merged commit 296063e into dani-garcia:master Oct 9, 2020
@rfwatson rfwatson deleted the add-db-connections-setting branch October 10, 2020 04:44
thelittlefireman pushed a commit to thelittlefireman/bitwarden_rs that referenced this pull request Mar 19, 2021
…setting

Add DATABASE_MAX_CONNS config setting
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.

5 participants