Skip to content

Commit

Permalink
Share the connection pool when there are multiple handlers
Browse files Browse the repository at this point in the history
In an application that has a primary and replica database the data
inserted on the primary connection will not be able to be read by the
replica connection.

In a test like this:

```
test "creating a home and then reading it" do
  home = Home.create!(owner: "eileencodes")

  ActiveRecord::Base.connected_to(role: :default) do
    assert_equal 3, Home.count
  end

  ActiveRecord::Base.connected_to(role: :readonly) do
    assert_equal 3, Home.count
  end
end
```

The home inserted in the beginning of the test can't be read by the
replica database because when the test is started a transasction is
opened byy `setup_fixtures`. That transaction remains open for the
remainder of the test until we are done and run `teardown_fixtures`.

Because the data isn't actually committed to the database the replica
database cannot see the data insertion.

I considered a couple ways to fix this. I could have written a database
cleaner like class that would allow the data to be committed and then
clean up that data afterwards. But database cleaners can make the
database slow and the point of the fixtures is to be fast.

In GitHub we solve this by sharing the connection pool for the replicas
with the primary (writing) connection. This is a bit hacky but it works.
Additionally since we define `replica? || preventing_writes?` as the
code that blocks writes to the database this will still prevent writing
on the replica / readonly connection. So we get all the behavior of
multiple connections for the same database without slowing down the
database.

In this PR the code loops through the handlers. If the handler doesn't
match the default handler then it retrieves the connection pool from the
default / writing handler and assigns the reading handler's connections
to that pool.

Then in enlist_fixture_connections it maps all the connections for the
default handler because all the connections are now available on that
handler so we don't need to loop through them again.

The test uses a temporary connection pool so we can test this with
sqlite3_mem. This adapter doesn't behave the same as the others and
after looking over how the query cache test works I think this is the
most correct. The issues comes when calling `connects_to` because that
establishes new connections and confuses the sqlite3_mem adapter. I'm
not entirely sure why but I wanted to make sure we tested all adapters
for this change and I checked that it wasn't the shared connection code
that was causing issues - it was the `connects_to` code.
  • Loading branch information
eileencodes committed Jan 3, 2019
1 parent fb6743a commit b24bfcc
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
23 changes: 23 additions & 0 deletions activerecord/lib/active_record/test_fixtures.rb
Expand Up @@ -173,10 +173,33 @@ def teardown_fixtures
end

def enlist_fixture_connections
setup_shared_connection_pool

ActiveRecord::Base.connection_handler.connection_pool_list.map(&:connection)
end

private

# Shares the writing connection pool with connections on
# other handlers.
#
# In an application with a primary and replica the test fixtures
# need to share a connection pool so that the reading connection
# can see data in the open transaction on the writing connection.
def setup_shared_connection_pool
writing_handler = ActiveRecord::Base.connection_handler

ActiveRecord::Base.connection_handlers.values.each do |handler|
if handler != writing_handler
handler.connection_pool_list.each do |pool|
name = pool.spec.name
writing_connection = writing_handler.retrieve_connection_pool(name)
handler.send(:owner_to_pool)[name] = writing_connection
end
end
end
end

def load_fixtures(config)
fixtures = ActiveRecord::FixtureSet.create_fixtures(fixture_path, fixture_table_names, fixture_class_names, config)
Hash[fixtures.map { |f| [f.name, f] }]
Expand Down
34 changes: 34 additions & 0 deletions activerecord/test/cases/fixtures_test.rb
Expand Up @@ -1362,3 +1362,37 @@ class NilFixturePathTest < ActiveRecord::TestCase
MSG
end
end

class MultipleDatabaseFixturesTest < ActiveRecord::TestCase
test "enlist_fixture_connections ensures multiple databases share a connection pool" do
with_temporary_connection_pool do
ActiveRecord::Base.connects_to database: { writing: :arunit, reading: :arunit2 }

rw_conn = ActiveRecord::Base.connection
ro_conn = ActiveRecord::Base.connection_handlers[:reading].connection_pool_list.first.connection

assert_not_equal rw_conn, ro_conn

enlist_fixture_connections

rw_conn = ActiveRecord::Base.connection
ro_conn = ActiveRecord::Base.connection_handlers[:reading].connection_pool_list.first.connection

assert_equal rw_conn, ro_conn
end
ensure
ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.connection_handler }
end

private

def with_temporary_connection_pool
old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base.connection_specification_name)
new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new ActiveRecord::Base.connection_pool.spec
ActiveRecord::Base.connection_handler.send(:owner_to_pool)["primary"] = new_pool

yield
ensure
ActiveRecord::Base.connection_handler.send(:owner_to_pool)["primary"] = old_pool
end
end

0 comments on commit b24bfcc

Please sign in to comment.