kenn / standby

Read from standby databases for ActiveRecord
MIT License
87 stars 28 forks source link

already initialized constant SlaverySlaveConnectionHolder #27

Closed caseyprovost closed 6 years ago

caseyprovost commented 6 years ago

After setting up a very basic slave in development I tried running a simple Slavery.on_slave{ User.count } and I get a recursive slavery-3.0.0/lib/slavery/connection_holder.rb:22: warning: already initialized constant SlaverySlaveConnectionHolder error

I am on rails 4.2.10 with ruby 2.3.3p222

kenn commented 6 years ago

cc @asanghi any insights? I haven't gotten around to upgrading yet.

caseyprovost commented 6 years ago

It might also be be good to have documentation around using raw SQL if it doesn't work like this:

result = Slavery.on_slave{ ActiveRecord::Base.connection.execute('SELECT * FROM users') }
asanghi commented 6 years ago

We have this in production and see no problems. I can investigate though. @caseyprovost can you post your database.yml? Your original example is pretty standard and should work.

kenn commented 6 years ago

The only case I can think of is that you are running a slave query from multiple threads at the same time, creating a race condition initializing Slavery.slave_connections.

Do you have something related? (e.g. config/initializers, before|after_fork in config/puma.rb, etc.)

caseyprovost commented 6 years ago

We use Unicorn and a failure standard configuration I think, which I shared below. I am sure it is just a MonkeyPatch I am missing or something.

default: &default
  adapter: mysql2
  encoding: utf8
  pool: 5
  username: *****
  password: *****
  host: 127.0.0.1
  port: 3306

development:
  <<: *default
  database: chewy_development

development_slave:
  <<: *default
  database: chewy_development_secondary

What is the downside of doing something like this instead?

class ReportObject < ApplicationRecord
  self.table_name = 'some_table'
  establish_connection(:development_slave)
end

ReportObject.connection.exec('SELECT COUNT(*) FROM some_table')

or

def on_slave(&block)
  databases = YAML.load(File.read('config/database.yml'))
  connection = ActiveRecord::Base.establish_connection(databases["#{Rails.env}_slave"])
  result = yield
  connection.disconnect!
  result
end
kenn commented 6 years ago

What is the downside of doing something like this instead?

You can do whatever you want, but this is an issue for the Slavery gem and don't treat us like your free consultant.

If necessary, reopen with answer to my questions please.

caseyprovost commented 6 years ago

@kenn I did answer the things asked for including posting my database.yml and sharing we are using unicorn and not Puma. I also cannot find any other evidence of forking.

I was also not attempting to be unfair or abuse your time, as an OS contributor myself I try to be aware and protective of that. My question was more accurately what is the advantage of Slavery to the approaches I mentioned? I am honestly curious.

Hopefully this was helpful.

kenn commented 6 years ago

You said

I am sure it is just a MonkeyPatch I am missing or something.

but didn't post that.

The other question is unrelated to the original problem and should go to a separate issue. To me the former looks ok but the latter is horrible - too much overhead. And by removing the overhead you'll basically end up creating the same thing as this gem. Remember, what this gem does is VERY MINIMAL.

caseyprovost commented 6 years ago

For the record the problem is the ActiveUUID gem. This library and that result in an infinite recursive loop.

kenn commented 6 years ago

I just installed activeuuid but it doesn't happen. Would appreciate if you can make a repro with a fresh new Rails setup.