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

max_open_connections default value is too low for the oracle database plugin #6790

Closed
meshantz opened this issue May 27, 2019 · 6 comments
Closed
Assignees
Milestone

Comments

@meshantz
Copy link

Is your feature request related to a problem? Please describe.
The max_open_connections setting for database plugins has a default value of 2, which is too low for the oracle database plugin. See hashicorp/vault-plugin-database-oracle#16

Describe alternatives you've considered
I see two options:

  1. Raise the default value to 3. However, I've seen issues related to too many file descriptors ("Too many open files" #1257 for instance) that reference this setting. So this solution could increase instances of such problems for users who aren't explicitly setting this value already.
  2. Allow the default value to be overridden on a per-plugin basis.

Describe the solution you'd like
I don't have a preference. I expect 1 is easier, but 2 will have a less wide-spread impact and allow opt-in rather than opt-out.

Feedback from those who have a better sense of the repercussions of one over the other would be nice.

@jefferai jefferai added this to the 1.1.3 milestone May 27, 2019
@jefferai
Copy link
Member

Happy to bump up the default from 2 (after all ideally this just works), but you can already override the default per mount -- https://www.vaultproject.io/api/secret/databases/oracle.html#max_open_connections

@meshantz
Copy link
Author

I understand you can override the value on the mount configuration. That works fine, but if you leave it on the default value for the oracle plugin, every mount you don't set this configuration on can get a deadlock waiting for an available connection.

I was thinking that bumping up the default for all database plugins might not be acceptable as a solution, so allowing each plugin to set the default would be an option.

If a higher default value is acceptable for all database plugins, then that's definitely the simplest solution.

@briankassouf briankassouf modified the milestones: 1.1.3, 1.2 Jun 4, 2019
@mjarmy
Copy link
Contributor

mjarmy commented Jun 6, 2019

What would a good default value be if we were to bump it up?

@jefferai
Copy link
Member

@mjarmy I think 4 would be good for now. If we go too high I'd be concerned that people might not be prepared for a potential tripling of connections.

4 across all database types as the new default seems sane, and we can document in CHANGES.

@mjarmy
Copy link
Contributor

mjarmy commented Jul 9, 2019

I've posted a draft PR that addresses this: #7093

@mjarmy mjarmy self-assigned this Jul 18, 2019
@mjarmy
Copy link
Contributor

mjarmy commented Jul 18, 2019

This is done, since #7093 has been merged

@mjarmy mjarmy closed this as completed Jul 18, 2019
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

No branches or pull requests

4 participants