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

Wrap Adapter enqueue methods and Batch callbacks with Rails Reloader; verify in tests that no Advisory locks remain at database connection check-in #1124

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Oct 21, 2023

Follow-up to #1121, #1122 and improved verification of #1113.

Notes about Rails Executors/Reloaders

These are the general details: https://guides.rubyonrails.org/threading_and_code_execution.html

Benefits / Risks

  • In a regular application—controller actions, jobs—I would expect these additional reloaders to be noops, because controller actions and jobs are already wrapped with executors/reloaders.
  • In tests, this might make tests slightly more reliable, if people are experiencing the same flakes that I am in GoodJob's test suite.
  • Introducing Reloaders/Executors does add additional potential for deadlocks, but given above, I largely expect them to be noops, or running inline in tests.

@bensheldon bensheldon force-pushed the checkin_locks branch 4 times, most recently from b287ff7 to b8d98eb Compare October 23, 2023 00:32
@bensheldon bensheldon changed the title Ensure no advisory locks exist when checking in db connections in Test (also spam Rails.application.executor.wrap in tests) Wrap Adapter enqueue methods and Batch callbacks with Rails Executor; verify in tests that no Advisory locks remain at database connection check-in Oct 23, 2023
@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Oct 23, 2023
… verify in tests that no Advisory locks remain at database connection check-in
@bensheldon bensheldon changed the title Wrap Adapter enqueue methods and Batch callbacks with Rails Executor; verify in tests that no Advisory locks remain at database connection check-in Wrap Adapter enqueue methods and Batch callbacks with Rails Reloader; verify in tests that no Advisory locks remain at database connection check-in Oct 23, 2023
@bensheldon bensheldon marked this pull request as ready for review October 23, 2023 15:26
@bensheldon bensheldon merged commit eea5fe9 into main Oct 23, 2023
20 checks passed
@bensheldon bensheldon deleted the checkin_locks branch October 23, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
Development

Successfully merging this pull request may close these issues.

1 participant