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

Increase stability by exposing DB connection and pool properties #324

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kentbull
Copy link

Type of change

  • Improvement (improvement to code, performance, etc)

Description

Exposed database connection properties so we can have automatic connection recycling in production for when connection pools go stale like when Google Cloud SQL does database maintenance about every quarter, sometimes more.

Additional details

Similar to the maxLifetime configuration property in the HikariCP database library, as well as others, providing access to these connection properties adds significant production stability by enabling connection pools to continually refresh themselves. Given that PostgreSQL connections don't garbage collect objects in SQL connections this configurability of connection pools is a requirement for long-term stability of the CA.

Related issues

#321

Release Note

Exposes operational database connection properties for ease of CA administration and stability.

@kentbull kentbull requested a review from a team as a code owner October 27, 2022 18:34
Copy link
Contributor

@jkneubuh jkneubuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kentbull , thanks for the submission. This seems like a really nice feature and well worth advancing. A couple of issues / questions:

  • The unit tests / validation tests are failing with the new interfaces, it looks like they are missing from some test mock APIs. Please could you make a pass at the tests, as there may be some lingering issues uncovered by the tests.

  • The behavior for default should not be to set the values as they are encoded in caconfig.go, but to omit the call to the SetXYZ method if the attribute is not specified in the input configuration. I.e... leave the behavior "as is," unless the config author has explicitly requested an override of the property.

@kentbull
Copy link
Author

Yes, I'll take another pass at the tests. I thought I got them all working, though I must have missed something.

And I agree, the default should be to leave the behavior "as is" unless the config author specifies desired properties.

I appreciate your input! I look forward to advancing this.

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.

2 participants