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

Fix thumbnail writing for multi-concurrent and multi-db setups #2433

Merged
merged 3 commits into from
Feb 27, 2023

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Feb 26, 2023

What is this pull request for?

Fix race condition in PictureThumb::Create

In a multi concurrent application server (like Puma) it happens that
a thumb might already exist for a given signature during the very
short timeframe of finding it vs creating it.

Using ARs create_or_find_by! do avoid ActiveRecord::RecordNotUnique errors. ActiveStorage does the
exact same thing to avoid concurrency issues.

Closes #2429

Connect to writing database during thumbnail generation

In a multi db setup where you have a reading and a writing database
we need to make sure that we are using the writing database.

This is necessary, because we are writing (caching) picture
thumbnails in a GET request. Usually Rails would handle this for us
on POST, PATCH/PUT and DELETE requests.

ActiveStorage does the same thing to support multi database
setups.

Closes #2428

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

This makes sure we only load from the DB if necessary.
Theoretically this would also happen with the former implementation,
but ActiveStorage is doing this as well, so I can't hurt to do it
explicilty.
In a multi concurrent application server (like Puma) it happens that
a thumb might already exist for a given signature during the very
short timeframe of finding it vs creating it.

Using ARs [create_or_find_by!](https://github.com/rails/rails/blob/8015c2c2cf5c8718449677570f372ceb01318a32/activerecord/lib/active_record/relation.rb#L218) do avoid ActiveRecord::RecordNotUnique errors. ActiveStorage does the
exact same thing to avoid concurrency issues.

Closes AlchemyCMS#2429
In a multi db setup where you have a reading and a writing database
we need to make sure that we are using the writing database.

This is necessary, because we are writing (caching) picture
thumbnails in a GET request. Usually Rails would handle this for us
on POST, PATCH/PUT and DELETE requests.

ActiveStorage does the same thing to support multi database
setups.

Closes AlchemyCMS#2428
@tvdeyen tvdeyen force-pushed the write-thumbs-on-writing-db branch from bdfd7aa to d47d2ef Compare February 26, 2023 13:38
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

¡Muy bueno!

@tvdeyen tvdeyen merged commit 11b0505 into AlchemyCMS:main Feb 27, 2023
@tvdeyen tvdeyen deleted the write-thumbs-on-writing-db branch February 27, 2023 16:07
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.

Duplicate key violations Do not write picture thumbnails on read-only replica databases
2 participants