thiagopradi / octopus

Database Sharding for ActiveRecord
2.53k stars 505 forks source link

Octopus.using nondeterministically selects shard if the block returns an ActiveRecord::Relation #481

Open sampriddy opened 6 years ago

sampriddy commented 6 years ago

Setup: I have one database on a shard named 'slave_read' where the database does not actually exist. Calls to this shard will throw an exception around the missing database name. Additionally, there is a master_shard referring to a real database.

In this first test, I asked Octopus to use the 'slave_read' shard which should always throw an error, passing it User.all as a query. As shown below it's inconsistent about which shard it actually sends the query to:

[22] pry(main)> Octopus.using(:slave_read) { User.all }.first
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[23] pry(main)> Octopus.using(:slave_read) { User.all }.first
[Shard: master_shard]  User Load (0.5ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
=> #<User:0x007f994052c840
 id: 766,
 name: "foo",
 created_at: Tue, 13 Mar 2018 17:19:22 UTC +00:00,
 updated_at: Tue, 13 Mar 2018 17:19:22 UTC +00:00,
 external_id: nil>
[24] pry(main)> Octopus.using(:slave_read) { User.all }.first
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[25] pry(main)> Octopus.using(:slave_read) { User.all }.first
[Shard: master_shard]  User Load (0.5ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
=> #<User:0x007f9940d8e948
 id: 766,
 name: "foo",
 created_at: Tue, 13 Mar 2018 17:19:22 UTC +00:00,
 updated_at: Tue, 13 Mar 2018 17:19:22 UTC +00:00,
 external_id: nil>

In a second test, if I do my .first call inside of the block so that a record is returned rather than a relation, and all requests go to the requested shard:

[33] pry(main)> Octopus.using(:slave_read) { User.all.first }
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[34] pry(main)> Octopus.using(:slave_read) { User.all.first }
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[35] pry(main)> Octopus.using(:slave_read) { User.all.first }
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[36] pry(main)> Octopus.using(:slave_read) { User.all.first }
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'
[37] pry(main)> Octopus.using(:slave_read) { User.all.first }
ActiveRecord::NoDatabaseError: FATAL:  database "fake_db_does_not_exist" does not exist
from /Users/spriddy/.gem/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:661:in `rescue in connect'

The difference seems to be around whether the final return value of the block is going to be a relation or a record. { User.all.to_a } is consistent but { User.all } is not.

I am on ActiveRecord 4.2.8 and ar-octopus 0.9.2.

sampriddy commented 6 years ago

Maybe this is the intended behavior? I do have two shards specified even if one is fake (so it would make sense to be about 50/50 if the intent is to load balance across replicated shards), and I see a spec that shows that requested shards are ignored when the query returns an ActiveRecord::Relation. I just wish that this was prominently called out in the readme at this point rather than discovering this after extensive debugging.

toao commented 5 years ago

Looks like this is still happening. Any updates?

serioushaircut commented 5 years ago

Just a hunch. 'to_s' or 'first' forces the query to be performed within the block, always triggering the error. A relation is lazily evaluated, not within the block, but later when the pry console does an implicit 'inspect' or whatever. That may then happen outside of the context of 'using'. (Every second time switching between master and slave).