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

Support prepending callbacks #30

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Support prepending callbacks #30

merged 3 commits into from
Feb 7, 2024

Conversation

quentindemetz
Copy link
Contributor

@quentindemetz quentindemetz commented Feb 5, 2024

Pass prepend: true to put the callback at the head of the callback chain.

In our application's case, the after_commit callback :foo was specified on a model, an instance of which is modified inside the transaction. We want our custom callback - registered later - to be executed before :foo

kudos @A1090 for the heavy lifting 🏋🏻

Copy link
Owner

@Envek Envek left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request! I have only one concern (see below).

And eager to learn more about your use case!

lib/after_commit_everywhere.rb Outdated Show resolved Hide resolved
@quentindemetz
Copy link
Contributor Author

quentindemetz commented Feb 6, 2024

And eager to learn more about your use case!

Here's a short explanation:

  • we're seeing significant load on our primary database and want to redirect as many SQL queries to our read replicas
  • we're already using rails multi-database support but would like to go further - because it doesn't handle queries coming from PUT/POST requests nor from Sidekiq jobs
  • we've patched activerecord internals to keep track, on a per-{request,job} basis, which tables have been recently written to
  • if a subsequent SELECT statement targets a table which was last written to more than <replication_lag> milliseconds ago, we execute that SELECT on the read replica
  • tables written to within a transaction need to be timestamped at the end of the transaction --> we use after_commit_everywhere for this
  • we have a problem where some other after_commit callbacks (defined on models) fire before our callback to register the writes. Code in those callbacks does not see any table write metadata, and incorrectly sends queries to the replica, where the data is not yet availablem, creating consistency problems 💥

Our initial benchmarks shows that this implementation holds promise:

  • on GET requests, 90% of SELECT queries are now run on the replica, up from 80% with the rails multi-database support
  • on non-GET requests, 32% of SELECT queries are now run on the replica, up from ~0%
  • on Sidekiq jobs, 50% of SELECT queries are now run on the replica, up from ~0%
  • application-wide, 85% of SELECT queries are now run on the replica, up from 75%

@Envek
Copy link
Owner

Envek commented Feb 7, 2024

Wow, that's a cool use case, thanks for sharing!

@Envek Envek merged commit d305caf into Envek:master Feb 7, 2024
10 checks passed
@Envek
Copy link
Owner

Envek commented Feb 7, 2024

Released in 1.4.0, please enjoy!

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.

3 participants