thiagopradi / octopus

Database Sharding for ActiveRecord
2.53k stars 505 forks source link

Octopus.using block doesn't use the right shard for relations #438

Open dvandersluis opened 7 years ago

dvandersluis commented 7 years ago

There seems to be a bug (using v0.9.0, Rails 4.2.8), when using the Octopus.using block form.

The following uses the master shard, despite shard1 being explicitly requested:

Octopus.using(:shard1) do
  users_with_posts = User.joins(:posts)
end

[Shard: master]  User Load (0.7ms)  SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id"
=> []

However, both of these use the correct shard:

Octopus.using(:shard1) do
  users_with_posts = User.joins(:posts).to_a
end

[Shard: shard1]  User Load (0.7ms)  SELECT  "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id"
=> [#<User:0x007fd6caade960 ...]
User.using(:shard1).joins(:posts)

[Shard: shard1]  User Load (0.7ms)  SELECT  "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id"
=> [#<User:0x007fd6caade960 ...]

It seems that if returning a Relation (or Octopus' proxy) within an Octopus.using block, it ignores the shard being requested. If it is converted from a Relation (using a method like to_a, first, etc.), it uses the correct shard.

zhangliwen commented 7 years ago

Hi, @dvandersluis, this problem has been solved? I have the same issues.

dvandersluis commented 7 years ago

Not that I know of.

kschutt commented 7 years ago

I'm having the same issue in v0.9.1 in Rails 5.0.4. I was able to reproduce your issue above, but it also similarly affects eager_load and includes.

We also use query objects in our codebase, and the using block doesn't seem to work for them either (below is an example). At first I thought it was octopus not liking these objects, but it seems like a deeper issue.

Octopus.using(:shard1) do 
  CurrentMonthUserQuery.new(Post.last.id).result
end
[Shard: master]  User Load (0.7ms)  SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" WHERE "users"."created_at" > '2017-10-01 00:00:00' AND "posts"."id" = 1
=> [#<User:0x000...]

But this does work (appending to_a to the query):

Octopus.using(:shard1) do 
  CurrentMonthUserQuery.new(Post.last.id).result.to_a
end
[Shard: shard1]  User Load (0.7ms)  SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" WHERE "users"."created_at" > '2017-10-01 00:00:00' AND "posts"."id" = 1
=> [#<User:0x000...]

where the CurrentMonthUserQuery is:

class CurrentMonthUserQuery
  def initialize(id)
    @post_id = id
  end

  def result
    User.joins(:posts).where('users.created_at > ? AND posts.id = ?', Time.current.beginning_of_month, @post_id)
  end
end
kschutt commented 7 years ago

@dvandersluis @zhangliwen I found a solution for ActiveRecord::Relation objects using Octopus::RelationProxy. I'm still working through the code to figure out why the proxy isn't being applied to ActiveRecord::Relation objects, but hopefully this helps in the meantime.

::Octopus::RelationProxy.new(:replica1, User.joins(:posts))
kschutt commented 7 years ago

Another solution is patching the proxy to use ShardTracking.

module Octopus
  class Proxy
    include ::Octopus::ShardTracking::Attribute
  end
end

module Octopus
  def self.using(shard, &block)
    conn = ActiveRecord::Base.connection

    if conn.is_a?(Octopus::Proxy)
      conn.current_shard = shard
      conn.run_on_shard(&block)
    else
      yield
    end
  end
end
swordfish444 commented 6 years ago

@thiagopradi Are you able to fix this?