Skip to content

Documentation for new Database Pooling configs #3

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

Open
wants to merge 17 commits into
base: impeller
Choose a base branch
from

Conversation

ChaelCodes
Copy link

@ChaelCodes ChaelCodes commented Jan 14, 2025

Motivation / Background

This Pull Request updates the guides to reflect the changes introduced in rails#54175.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@matthewd matthewd force-pushed the impeller branch 3 times, most recently from 51a24da to 92f0a45 Compare January 20, 2025 13:27
matthewd and others added 16 commits January 21, 2025 14:48
We should still avoid using them whenever possible, but e.g. I'd like to
be able to break connections inside a pool test.
The shared setup method creates a connection pool, and having the
isolation level change after a pool is created makes things awkward.

This has slightly more duplication, but not enough that I feel the need
to seek a different abstraction.
In the process, be more consistent about always disconnecting pools
we've created.
Building upon the "checkout for maintenance" primitives, this gives us a
method to progressively loop over idle connections without the risk of
starving the pool while we work.

Even for something internal, I don't love the API... but hopefully it
will suffice until we come up with something better.
Optionally ensure the full database connection chain sees regular query
traffic (without affecting our internal idle counters).
Avoid using database connections that were originally established over
the configured duration ago. This can be helpful to provide smooth
failover between connection proxies.
This is ideal where the caller knows that a database server/proxy
failover is occurring (meaning that previously-established connections
are now potentially pointing to the wrong place, and new fresh
connections will go to the right one); where such signalling is
possible, it can be used in place of an onerously low / "preventative"
max_age.
It's less elegant, but just seems easier to reason about.
@ChaelCodes ChaelCodes changed the title Documentation and Deprecation for new Database Pooling configs Documentation for new Database Pooling configs Jan 21, 2025
Comment on lines +3597 to +3604
<!-- Suggestions please -->
Idle database connections can be bad - the database may no longer exist, or
may assume the application server no longer exists. `keepalive` will check
open database connections and keep them alive.

<!-- How does this interact with idle_timeoue? -->
<!-- How does this interact with min_connections? -->
<!-- How does this interact with INFINITY? -->
Copy link
Author

Choose a reason for hiding this comment

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

cc @matthewd

Hi, could you kindly offer some suggestions or thoughts on how these configurations interact with each other?

Maybe some of these details could/should be moved to https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html ?

@matthewd matthewd force-pushed the impeller branch 4 times, most recently from c184e9a to b0ec072 Compare February 25, 2025 20:04
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