AlchemyCMS / alchemy_cms

Alchemy is the Open Source Rails CMS framework for the component based web that can be used as classic server side rendered or headless CMS.
https://www.alchemy-cms.com
BSD 3-Clause "New" or "Revised" License
846 stars 314 forks source link

Do not write picture thumbnails on read-only replica databases #2428

Closed dssjoblom closed 1 year ago

dssjoblom commented 1 year ago

Steps to reproduce

After upgrading from an older 6.0.0 prerelease version to 6.1.1, it seems like when an image is rendered, it causes database writes:

0:41:52 rails.1   | [127.0.0.1] Write query attempted while in readonly mode: INSERT INTO "alchemy_picture_thumbs" ("picture_id", "signature", "uid") VALUES ($1, $2, $3) RETURNING "id"
10:41:52 rails.1   | [127.0.0.1] /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/postgresql_adapter.rb:645:in `execute_and_clear'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/postgresql/database_statements.rb:55:in `exec_query'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:136:in `exec_insert'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/postgresql/database_statements.rb:93:in `exec_insert'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:171:in `insert'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/query_cache.rb:22:in `insert'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/persistence.rb:375:in `_insert_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/persistence.rb:929:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/counter_cache.rb:166:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/locking/optimistic.rb:79:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/attribute_methods/dirty.rb:201:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/callbacks.rb:461:in `block in _create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/callbacks.rb:98:in `run_callbacks'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/callbacks.rb:824:in `_run_create_callbacks'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/callbacks.rb:461:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/timestamp.rb:108:in `_create_record'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/persistence.rb:900:in `create_or_update'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/callbacks.rb:457:in `block in create_or_update'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/callbacks.rb:106:in `run_callbacks'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/callbacks.rb:824:in `_run_save_callbacks'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/callbacks.rb:457:in `create_or_update'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/timestamp.rb:126:in `create_or_update'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/persistence.rb:507:in `save!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/validations.rb:53:in `save!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/transactions.rb:302:in `block in save!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/transactions.rb:354:in `block in with_transaction_returning_status'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `block in transaction'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/transaction.rb:319:in `block in within_new_transaction'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activesupport-6.1.7.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/transaction.rb:317:in `within_new_transaction'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `transaction'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/transactions.rb:350:in `with_transaction_returning_status'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/transactions.rb:302:in `save!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/suppressor.rb:48:in `save!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/activerecord-6.1.7.1/lib/active_record/persistence.rb:55:in `create!'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/alchemy-dragonfly-s3-6.0.0/lib/alchemy/dragonfly/s3/create_picture_thumb.rb:12:in `call'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/alchemy/picture/url.rb:39:in `uid'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/gems/alchemy-dragonfly-s3-6.0.0/app/models/alchemy/picture/s3_url.rb:9:in `call'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/alchemy/picture.rb:180:in `url'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/concerns/alchemy/picture_thumbnails.rb:40:in `picture_url'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/alchemy/essence_picture_view.rb:55:in `src'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/alchemy/essence_picture_view.rb:60:in `img_tag'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/models/alchemy/essence_picture_view.rb:31:in `render'
10:41:52 rails.1   | /home/daniel/.rvm/gems/ruby-3.1.3/bundler/gems/alchemy_cms-cadcb5a23b09/app/views/alchemy/essences/_essence_picture_view.html.erb:6:in `__home_daniel__rvm_gems_ruby_______bundler_gems_alchemy_cms_cadcb_a__b___app_views_alchemy_essences__essence_picture_view_html_erb___559467133831684211_190640'

Expected behavior

I'd expect no database writes on a GET query (the Rails default when using read-only replicas).

Actual behavior

Alchemy apparently tries to create thumbnails (or something similar) while rendering the picture view.

System configuration

tvdeyen commented 1 year ago

This was introduced in Alchemy 5.1

Please see https://github.com/AlchemyCMS/alchemy_cms/pull/1882/ for why we introduced it.

ActiveStorage does a similar thing. Writing a cache in a GET request is a common thing.

Not sure about the read only replica. Maybe there is a way to restrict those writes on the writing database somehow?

Want to look into it?

dssjoblom commented 1 year ago

@tvdeyen yes, it happens that you sometimes have to write to the database even for a GET request.

The default setup is this: (from https://guides.rubyonrails.org/active_record_multiple_databases.html#activating-automatic-role-switching):

If the application is receiving a POST, PUT, DELETE, or PATCH request the application will automatically write to the writer database. For the specified time after the write, the application will read from the primary. For a GET or HEAD request the application will read from the replica unless there was a recent write.

Switching to the writing role from e.g. a GET can be done like this:

ActiveRecord::Base.connected_to(role: :writing) do
  # explicitly write to the primary 
end

So this is a pretty simple fix, except I don't know where exactly to put the block in AlchemyCMS. In the application I'm working on, I simply surrounded each alchemy template render with something like this:

<% if AppConfig.cms_blocks_enabled %>
  <% alchemy_page = Alchemy::Page.find_by(page_layout: block_name) %>
  <% if alchemy_page %>
    <div id="<%= id %>" class="<%= classes %>">
      <%# for some reason, images can be written during render %>
      <% ActiveRecord::Base.connected_to(role: :writing) do %>
        <%= render_alchemy_elements from_page: alchemy_page %>
      <% end %>
    </div>
  <% end %>
<% end %>

You can see the same approach in ActiveStorage: https://github.com/rails/rails/blob/75a9e1be75769ae633a938d81d51e06852a69ea3/activestorage/app/models/active_storage/variant_with_record.rb

def create_or_find_record(image:)
      @record =
        ActiveRecord::Base.connected_to(role: ActiveRecord.writing_role) do
          blob.variant_records.create_or_find_by!(variation_digest: variation.digest) do |record|
            record.image.attach(image)
          end
        end
    end

Coincidentally, the AS code also uses the create_or_find_by! idiom to prevent race conditions (#2429).

tvdeyen commented 1 year ago

So this is a pretty simple fix, except I don't know where exactly to put the block in AlchemyCMS. In the application I'm working on, I simply surrounded each alchemy template render with something like this:

Thanks. I think the best place for the ActiveRecord::Base.connected_to(role: :writing) block would be somewhere around here https://github.com/AlchemyCMS/alchemy_cms/blob/0e13375c362a60d1e65eec05f4c357a440fddff8/app/models/alchemy/picture/url.rb#L39

You can see the same approach in ActiveStorage: https://github.com/rails/rails/blob/75a9e1be75769ae633a938d81d51e06852a69ea3/activestorage/app/models/active_storage/variant_with_record.rb

Coincidentally, the AS code also uses the create_or_find_by! idiom to prevent race conditions (https://github.com/AlchemyCMS/alchemy_cms/issues/2429).

We are using a similar approach, but maybe the detect is wrong here and we should explicitly use find_by as well in order to make sure we read from the database and not an in-memory collection. We used to detect, because we wanted to avoid database reads in case of an eager loaded collection. But if you think about it, it does not make sense to eager load thumbs, if they are not used anyway (because the elements are cached most of the time)

Will play around a bit.

tvdeyen commented 1 year ago

@dssjoblom would please give

# Gemfile
gem "alchemy_cms", git: "https://github.com/tvdeyen/alchemy_cms", branch: "6.1-write-thumbs-on-writing-db"

a try?

dssjoblom commented 1 year ago

@tvdeyen yes, this seems to work :thumbsup: Thanks a lot!