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

Restore connection pools after transactional tests #40384

Merged
merged 1 commit into from Apr 21, 2021

Conversation

eugeneius
Copy link
Member

Since #34773, all connection handlers share the primary connection pool during transactional tests to ensure that replica connections can read data written to primary connections.

The replica connection pools were never restored, which meant that the behaviour of non-transactional tests depended on whether a transactional test had run first, which could result in nondeterministic failures.

@eugeneius eugeneius force-pushed the teardown_shared_connection_pool branch from a27124f to 02425a8 Compare October 14, 2020 12:36
eileencodes added a commit to seejohnrun/rails that referenced this pull request Oct 21, 2020
Things pass except for the fixture connection tests and one test that
is getting too many shards in the list because we're no longer cleaning
up connections at the end of the tests.

We need a way I think to turn off shared connection pools for the AR
tests, or at least when we're not doing multi-connection work.

rails#40384 is interesting but I didn't
have time to make it work with our stuff.

So basically we're in a better state but I'm not quite sure yet how to
get just the 2 tests that need shared pools to use them. Thoughts?
@eugeneius eugeneius force-pushed the teardown_shared_connection_pool branch from 02425a8 to 5fb3066 Compare October 30, 2020 03:06
Since b24bfcc, all connection handlers
share the primary connection pool during transactional tests to ensure
that replica connections can read data written to primary connections.

The replica connection pools were never restored, which meant that the
behaviour of non-transactional tests depended on whether a transactional
test had run first, which could result in nondeterministic failures.
@eugeneius eugeneius force-pushed the teardown_shared_connection_pool branch from 5fb3066 to 054e19d Compare November 15, 2020 22:50
Base automatically changed from master to main January 14, 2021 17:02
@rails-bot
Copy link

rails-bot bot commented Apr 14, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Apr 14, 2021
@rails-bot rails-bot bot closed this Apr 21, 2021
@senny
Copy link
Member

senny commented Apr 21, 2021

I think we hit this problem during our upgrade. @eugeneius do you know if the patch is still necessary?

@eugeneius
Copy link
Member Author

Yep, we still have workarounds for this problem in our app that I'd like to remove. I'll merge and backport this now.

@eugeneius eugeneius reopened this Apr 21, 2021
@rails-bot rails-bot bot removed the stale label Apr 21, 2021
@eugeneius eugeneius merged commit 19a4bfd into rails:main Apr 21, 2021
eugeneius added a commit that referenced this pull request Apr 21, 2021
Restore connection pools after transactional tests
eugeneius added a commit that referenced this pull request Apr 21, 2021
Restore connection pools after transactional tests
@eugeneius
Copy link
Member Author

Backported to 6-1-stable in e6192c1 and 6-0-stable in 3574a1b. Thanks for the reminder @senny!

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

Successfully merging this pull request may close these issues.

None yet

2 participants