Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow prepared_statements: false in database.yml #110

Closed
gregburek opened this issue Jun 21, 2013 · 15 comments
Closed

Allow prepared_statements: false in database.yml #110

gregburek opened this issue Jun 21, 2013 · 15 comments
Labels

Comments

@gregburek
Copy link

Starting with AR 3.1, prepared_statements are default on when interacting with the db. Named prepared statements disallow transaction pooling with tools such as pgbouncer. Additionally, Postgres before 9.2 may experience a performance regression with certain prepared statement parameters as only the initial query plan is used for the lifetime of a prepared statement. Some paramaters would be better served by different query plans and will suffer when run.

Rails documentation change: rails/rails@c666323

Rails open issue re pgbouncer: rails/rails#1627

I'm a bit at a loss on how to trigger the inclusion of prepared_statements: false during the database.yml injection, otherwise a PR would be attached. Do you guys have a better idea about this?

@schneems
Copy link
Contributor

I would have users of the buildpack be aware of the issue and set the flag in code in an initializer similar to how they can configure db pool size here:

https://devcenter.heroku.com/articles/concurrency-and-database-connections

@gregburek
Copy link
Author

So, something like this?

config = Rails.application.config.database_configuration[Rails.env]
config['reaping_frequency']  = ENV['DB_REAP_FREQ'] || 10 # seconds
config['pool'] = ENV['DB_POOL'] || 5
config['prepared_statements']  = ENV['DB_PREPARED_STATEMENTS'] || false
ActiveRecord::Base.establish_connection(config)

@daveyeu
Copy link

daveyeu commented Jun 25, 2013

Wouldn't the above need to be repeated wherever connections are made? Specifically I'm thinking Unicorn and Resque after_fork blocks.

@bdotdub
Copy link

bdotdub commented Jun 25, 2013

You can add anything to the database.yml config if you add it to the end of the DATABASE_URL, ie:

postgres://user:pass@your.compute-1.amazonaws.com:5432/database?prepared_statements=false

This line in the Ruby buildpack walks through each of the URI params and adds them to the config file: https://github.com/heroku/heroku-buildpack-ruby/blob/master/lib/language_pack/ruby.rb#L548

@daveyeu
Copy link

daveyeu commented Jun 25, 2013

@bdotdub Doesn't seem to work for me. I added a test endpoint to give me the database config, and it yields:

{
  :adapter             => "postgresql", 
  :username            => "hvhhnhfzwaxyzv", 
  :password            => "U3ZeMl695xlMqDJeftjaQ4JJsK", 
  :port                => 6000, 
  :database            => "d8bjp5loq05526", 
  :host                => "127.0.0.1", 
  :prepared_statements => "false"
}

It isn't casting the 'false' to a boolean. Sad face.

@bdotdub
Copy link

bdotdub commented Jun 25, 2013

Weird 🐧

This is what I'm getting in my env:

irb(main):022:0> ENV["DATABASE_URL"]
=> "postgres://<user>:<password>@127.0.0.1:6000/database?prepared_statements=false"

irb(main):023:0> pp Rails.application.config.database_configuration[Rails.env]
{"adapter"=>"postgresql",
 "database"=>"database",
 "username"=>"<user>",
 "password"=>"<password>",
 "host"=>"127.0.0.1",
 "port"=>6000,
 "prepared_statements"=>false}

@catsby
Copy link
Contributor

catsby commented Jun 25, 2013

Adding to the DATABASE_URL is not recommended. While it works, it's fragile (will be lost if you rotate credentials, promote a new database, etc).

@daveyeu
Copy link

daveyeu commented Jun 25, 2013

@bdotdub Hrm. Maybe I got it wrong. Double-checking...

@gregburek
Copy link
Author

@daveyeu FYI Your db creds are in plain text above. Roll your db creds asap: heroku pg:credentials --reset

This situation is a good example of why adding config to your DATABASE_URL is a bad idea. If the creds change, the config var will be overwritten and all additions lost.

@daveyeu
Copy link

daveyeu commented Jun 25, 2013

@gregburek Thanks for the heads up. Rotated.

While altering DATABASE_URL is perilous, @bdotdub pointed out that updating Procfile works in this case:

web: bin/pgbouncer-stunnel.sh && DATABASE_URL=$PGBOUNCER_URI?prepared_statements=false ...

I'm still seeing inconsistencies, however:

Rails.application.config.database_configuration[Rails.env][:prepared_statements] # => false
ActiveRecord::Base.connection_pool.instance_variable_get('@config')[:prepared_statements] # => "false"

:garbage:

@catsby
Copy link
Contributor

catsby commented Jun 25, 2013

@daveyeu What does ActiveRecord::Base.connection_config report?

@daveyeu
Copy link

daveyeu commented Jun 25, 2013

@catsby I had to scuttle the branch so something could get QA'd. I'll take a look later.

From what I can tell, however, it looks like ActiveRecord::Base.establish_connection (the call in Unicorn's after_fork block) uses ENV['DATABASE_URL'] by default, and parses it without any casting. Supplying a config hash directly looks like it would resolve it.

@catsby
Copy link
Contributor

catsby commented Jun 25, 2013

@daveyeu yes, @gregburek and I confirmed that today and I'll be updating the Dev Center article Richard mentioned to address this. The answer to your original question is "yes" then, if you're using Unicorn you'll want to do this configuration setup in the after_fork block. You'll likely not want/need the initializer in that case.

@gregburek
Copy link
Author

I think I also figured this out for my pgbouncer buildpack in this commit. It isn't pretty, but since I am already rewriting DATABASE_URL on dyno launch, it allows for further appending and should work across promotes and changeovers.

Thanks to @daveyeu for the inspiration.

@schneems
Copy link
Contributor

we now no longer over-write database.yml in Rails 4.1+ so, done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants