Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

rake db:test:prepare destroys test schema when development shards are defined #31

Closed
chrisb87 opened this Issue Apr 4, 2011 · 32 comments

Comments

Projects
None yet

chrisb87 commented Apr 4, 2011

With the following shards.yml:

octopus:
  replicated: true
  fully_replicated: true

  environments:
      - production
      - development

  development:
    slave1:
      adapter: mysql2
      encoding: utf8
      reconnect: false
      database: cornice_development
      pool: 5
      username: root
      password: 
      #socket: /tmp/mysql.sock
      host: localhost

  production:
    slave1:

My test database is wiped clean, including the schema, when I try to the rspec test suite or "rake db:test:prepare". If I remove the development entries from shards.yml, db:test:prepare correctly loads the schema in schema.rb into the test database.

chrisb87 commented Apr 8, 2011

I've created a minimal Rails project to demonstrate the issue: https://github.com/chrisb87/octopus-bug

Owner

thiagopradi commented Apr 8, 2011

Nice, I'm investigating it.

thoddes commented Sep 19, 2011

I'm having the same issue. Has there been any progress on a solution?

Contributor

xaviershay commented Oct 21, 2011

Here is a workaround. Drop in in lib/tasks/whatever.rake:

# This is to work around an octopus bug, where it does not respect calls to ActiveRecord::Base.establish_connection.
# db:schema:load relies on this when used in tasks such as db:test:prepare which dynamically reconnect to different
# environment's databases.
#
# https://github.com/tchandy/octopus/issues/31
task :reconnect_octopus do
  if ActiveRecord::Base.is_a?(Octopus::Proxy)
    config = ActiveRecord::Base.connection.config
    ActiveRecord::Base.connection.initialize_shards(config)
  end
end
task :'db:schema:load' => :reconnect_octopus

fabn commented Dec 7, 2011

@xaviershay that hasn't worked for me.

Is there any chance that this issue will be fixed?

Contributor

xaviershay commented Dec 7, 2011

Argh I totally pasted the wrong thing, needs to check ActiveRecord::Base.connection. Anyway we found a better workaround:

if ActiveRecord::Base.connection.is_a?(Octopus::Proxy)
  ActiveRecord::Base.connection.initialize_shards(Octopus.config)
end

fabn commented Dec 7, 2011

It does work, thanks.

Owner

thiagopradi commented Dec 14, 2011

@xaviershay any chance of a patch loading this using Rails railties?

Contributor

xaviershay commented Dec 15, 2011

I'm pretty flat out atm, maybe over the Christmas break.

Just wanna confirm, I'm having the same issue :-)

It seems like this doesn't work with config.active_record.schema_format = :sql. Any ideas how to fix it?

@JonasNielsen, that is most likely caused by the fact that db:test:prepare uses db:test:clone_structure instead of db:test:load when the schema_format is sql.

https://github.com/rails/rails/blob/3-0-stable/activerecord/lib/active_record/railties/databases.rake#L471

Thanks, @joslynesser, that makes sense.

Is it possible to fix the issue for sql schema format as well?

This does not work:

task :reconnect_octopus do
  if ActiveRecord::Base.connection.is_a?(Octopus::Proxy)
    ActiveRecord::Base.connection.initialize_shards(Octopus.config)
  end
end
task :'db:test:clone_structure' => :reconnect_octopus

@JonasNielsen, which version of rails are you using? It looks like there has been additional changes since Rails3, such as changing db:test:clone_structure to depend on a new db:structure:load: rails/rails@15fb430

I'm currently stuck on a production Rails 3.0 app at the moment, but maybe try something like this instead?

task :reconnect_octopus do
  if ActiveRecord::Base.connection.is_a?(Octopus::Proxy)
    ActiveRecord::Base.connection.initialize_shards(Octopus.config)
  end
end
task :'db:structure:load' => :reconnect_octopus

Thanks, @joslynesser. We are running Rails 3.1.3.
I tried the above, but without luck.

$ bundle exec rake spec
autouncle_dk_test already exists
psql: warning: extra command-line argument "template0" ignored
psql: FATAL:  database "autouncle_dk_test" does not exist
/Users/Jonas/.rvm/rubies/ruby-1.9.2-p290-gcpatch/bin/ruby -S rspec ./spec/lib/car_subscription_catalog_spec.rb (...)
No DRb server is running. Running in local process instead ...
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}


Finished in 0.38086 seconds
0 examples, 0 failures
/Users/Jonas/.rvm/gems/ruby-1.9.2-p290-gcpatch/gems/activerecord-3.1.3/lib/active_record/connection_adapters/postgresql_adapter.rb:1076:in `initialize': FATAL:  database "autouncle_dk_test" does not exist (PGError)

It deletes the database, but doesn't create a new one. Our schema format is :sql btw.

@JonasNielsen, it looks like it might be a different issue in the underlying db:test:prepare task.

After further testing, I've confirmed that a few standard ActiveRecord database tasks are broken with Octopus. The create_database method that is used in a few tasks (such as db:create, db:test:purge) uses ActiveRecord::Base.establish_connection and ActiveRecord::Base.connection, which will return the connection proxy instead of the intended connection. Because the proxy is returned, an error is thrown. Due to the generic rescue in create_database, it will output database already exists even though it does not exist in this case.

As a temporary fix, you can ensure that octopus uses the standard connection for these tasks, as shown here:

task :use_standard_connection do
  ActiveRecord::Base.write_inheritable_attribute(:establish_connection, true)
end
task :'db:create' => :use_standard_connection
task :'db:test:purge' => :use_standard_connection

For those curious as to why the proxy is returned and not the intended connection, the code is here: https://github.com/tchandy/octopus/blob/master/lib/octopus/model.rb#L54-56

I believe this problem is related to issue #42

Anyone have ideas on how this should be fixed within octopus?

To fix this using rails 3.2 merge pull 94 and add this to rake file instead

task :use_standard_connection do
  ActiveRecord::Base.custom_octopus_connection = true
  ActiveRecord::Base.establish_connection
end
task :'db:create' => :use_standard_connection
task :'db:test:purge' => :use_standard_connection

england commented Jul 24, 2012

privet, in Rails 3.0.5 & ar-octopus (0.3.4) it works with

task :reconnect_octopus do
  if ActiveRecord::Base.connection.is_a?(Octopus::Proxy)
    ActiveRecord::Base.connection.initialize_shards(Octopus.config)
  end
end
task :'db:schema:load' => :reconnect_octopus
Collaborator

sobrinho commented Aug 4, 2012

Can anyone do a pull request including a test case?

/cc @xaviershay @england

Contributor

xaviershay commented Aug 4, 2012

I'm not going to to do this sorry: too long ago, I don't remember enough context, and many other things on my list today.

Collaborator

sobrinho commented Aug 4, 2012

@xaviershay no problem!

I'm closing this issue because I could not reproduce this problem.

Please reopen if still an issue.

@sobrinho sobrinho closed this Aug 4, 2012

Collaborator

sobrinho commented Sep 8, 2012

I'm reopening this issue because I did something different this time and the issue happened.

I configured two shard for development environment and rake db:test:prepare didn't prepared the database.

@sobrinho sobrinho reopened this Sep 8, 2012

Exactly! :-) and again - the temp fix I wrote about above works for fixing it - but we should definitely find the root cause.

Collaborator

sobrinho commented Sep 22, 2012

In short, db:test:prepare is calling establish_connection right here but octopus don't know what to do right here.

I'm trying to figure out why the hell octopus is being activated in test environment.

Okay, let me know if I can do something :-)

Collaborator

sobrinho commented Jan 26, 2013

Closing since 54bf493 fixed this issue, thanks @ragalie!

@sobrinho sobrinho closed this Jan 26, 2013

Collaborator

ragalie commented Jan 26, 2013

We should probably put a test around this though. I'll plan on doing that when I write tests for the other db tasks.

Collaborator

sobrinho commented Jan 26, 2013

It was related to calling establish_connection instead of octopus_establish_connection, which we can't do inside rails codebase.

Since we removed octopus_establish_connection and fixed establish_connection, I'm happy with the test covering the establish_connection behavior.

You think something else is necessary?

Collaborator

ragalie commented Jan 26, 2013

The thing that worries me is that a lot of the rake task-related tests right now rely on the implementation staying the same. For instance, the reason that I didn't catch the typo fixed in #150 is that all of the migration tests call ActiveRecord::Migrator.run, but db:rollback calls .rollback, which eventually calls .down, which had a typo.

I think it would be safest to test that running the db:rollback task (or db:test:prepare, in this case) results in the expected behavior.

Collaborator

sobrinho commented Jan 26, 2013

Ok, makes sense.

We're still having this problem as of 0.6 with Rails 3.0.17, fixed by:

task :reconnect_octopus do
  if ActiveRecord::Base.connection.is_a?(Octopus::Proxy)
    ActiveRecord::Base.connection.initialize_shards(Octopus.config)
  end
end
task :'db:schema:load' => :reconnect_octopus

Is this worth reopening or should I assume we need to get to Rails 3.1 or 3.2?

I'm seeing a similar issue using the fixture_builder gem, which defines the following rake task:

    desc "Build the generated fixtures to spec/fixtures if dirty"
    task :build => :environment do
      ActiveRecord::Base.establish_connection('test')
      Dir.glob(File.join(Rails.root, '{spec,test}', '**', 'fixture_builder.rb')).each{|file| require(file)}
    end

In my environment, I don't define the key 'test' in my shards.yml file (it comes from database.yml file), but I have set up shards for my development environment. By the time the this rake task has run, octopus has already hijacked the connection, and the connection ultimately does not switch to the test database, which results in the development database getting truncated. I can't find an an easy workaround for this. Maybe the rake tasks expectation of being able to change the database connection isn't reasonable, but if this is how establish_connection is supposed to work, then I think that octopus may need to respect it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment