BufferedMutation breaks after_xxx hooks.

curi1119 commented 2 years ago

Problem

activerecord-spanner-adapter breaks ActiveRecord's after_xxx hooks. Perhaps this is a problem caused by BufferedMutation.

Like codes shows in bellow, I am unable to retrieve the record in the after_save block immediately after insertion.

I doubt that this similar problem is also happening in after_update and after_create.

Solution(?)

I do not have a idea to monkey-patch for this problem. My only solution was to remove require "activerecord_spanner_adapter/base" from lib. https://github.com/googleapis/ruby-spanner-activerecord/blob/07f5b6cbfc701848e3aebf4cb195d267d9e52fb8/lib/active_record/connection_adapters/spanner_adapter.rb#L23

Reproduce

sample code:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'activerecord', '6.1.6'
  gem 'activerecord-spanner-adapter'
  # gem 'activerecord-spanner-adapter',
  #   git: 'https://github.com/googleapis/ruby-spanner-activerecord',
  #   ref: '2d30298fd0dedb447ce9ff089df43c65131dc9fe' # composite-pk branch
end

require "active_record"
require "activerecord-spanner-adapter"
require "google/cloud/spanner"

spanner = Google::Cloud::Spanner.new project: "test-project", emulator_host: "localhost:9010"
job = spanner.create_instance "test-instance",
                              name: "Test Instance",
                              config: "emulator-config",
                              nodes: 1
job.wait_until_done!

instance = spanner.instance "test-instance"
job = instance.create_database "testdb", statements: []
job.wait_until_done!

ActiveRecord::Base.establish_connection(project: "test-project", adapter: "spanner", instance: "test-instance", database: "testdb", emulator_host: "localhost:9010")

ActiveRecord::Schema.define do
  create_table :singers, force: true, id: false do |t|
    t.integer :singer_id, null: false, primary_key: true
    t.column :first_name, :string, limit: 200
    t.string :last_name
  end
end

class Singer < ActiveRecord::Base
  after_save do
    Singer.print_name
  end

  def self.print_name
    puts "first name => #{Singer.first&.first_name}"
  end
end

Singer.create(first_name: "John", last_name: "Doe")
Singer.print_name
singer = Singer.first
singer.first_name = "Jane"
singer.save!

output:

-- create_table(:singers, {:force=>true, :id=>false})
   -> 0.0135s
first name =>           <=== can't see created record in after_save
first name => John
first name => Jane

This is expected output by using MySQL or remove require "activerecord_spanner_adapter/base" from lib.

-- create_table(:singers, {:force=>true, :id=>false})
   -> 0.0172s
first name => John
first name => John
first name => Jane

olavloite commented 2 years ago

@curi1119 I see now what you are doing, and yes; that is caused by buffered_mutations. I'm however inclined to say that it is a 'known limitation' of the Spanner adapter. The reason that it does not work is that the code you have in the after_save method tries to re-fetch the record from the database:

  def self.print_name
    puts "first name => #{Singer.first&.first_name}"
  end

(The Singer.first call goes to the database to fetch the record)

Instead, in this specific example, you could just call first_name on the Singer instance that you have in the after_save method. There might be specific use cases where it could make sense to fetch the record from the database, for example if you have a column that is generated by the database. The workaround in that case would be to explicitly use a transaction so the Spanner provider does not use mutations.

The reason that the record is not found when you try to query the database in the after_save method is that the transaction has not yet been committed. Mutations are only sent to the database when the transaction is committed. A workaround for your specific example could therefore also be to replace the after_save callback with an after_commit callback. At that point the record will be available in the database.

curi1119 commented 2 years ago

@olavloite

Thank you for reply.

Yes, I understand limitation of buffered_mutation and workaround for this problem. I might use after_commit or transaction to avoid using buffered_mutation for this problem. (My example is just simplified to show the problem)

My concern is that buffered_mutations changes ActiveRecord's default behavior.

It might be useful to solve avoid limitation of buffered_mutation if there is an implementation like to follow:

completely turn off buffered_mutations manually.
- fall back to AR default behavior.
- currently, transaction block is the only way to code.
- have to remove activerecord_spanner_adapter/base from lib to completely turn off.
detect callback, so that not to use buffered_mutation for some cases
- not sure this to adapt to the most cases

googleapis / ruby-spanner-activerecord