sdsykes / slim_scrooge

SlimScrooge heavily optimises your database interactions
313 stars 25 forks source link

Collisions and logic #25

Closed Vittly closed 10 years ago

Vittly commented 11 years ago

Hello! We use scrooge (1.0.12) in large-production-rails-application and sometimes (twice a week)) we got MySQL "unknown columns" exception becouse of your hash-based algorithm of callsites keys calculation. May be you need to improve it. Another case is about logic. Slimscrooge 1st time loads full objects from database and then with monitored_hash tries to calculate what columns were needed. 2nd time it loads only these columns. But may be you should not load 1st time large columns (blobs, texts) and reload them if needed. It would be better logic in cases of large amount of result rows.

sdsykes commented 11 years ago

It's surprising that you get unknown columns given that the callsite hash calculation uses the part of the select query that contains the table names. Do you have logs of what type of queries cause these problems?

I'm not sure what the logic you would wish for the first time code. If you don't load any columns, then it is very likely you will always need to reload. In this case scrooge reloads all columns to avoid making a query for every new columns that is required. Particularly the first time through this is usually desirable, but seeing as we know we have to reload anyway, you may as well load all columns at the start.

If you are suggesting we determine what to load initially partly by column type, that is certainly an enhancement that could be considered. Patch welcome!

Vittly commented 11 years ago

Log part you wanted: Mysql::Error: Unknown column 'tags.name' in 'field list': DB somehost/sommedatabase -> SELECT tags.name,tags.elements_count,tags.id,tags.uri_name,tags.site_id FROM sites WHERE (sites.id = 8) /opt/ruby-ent/lib/ruby/gems/1.8/gems/slim_scrooge-1.0.12/lib/slim_scrooge/slim_scrooge.rb:31:in find_with_callsite_key' /opt/ruby-ent/lib/ruby/gems/1.8/gems/slim_scrooge-1.0.12/lib/slim_scrooge/slim_scrooge.rb:17:infind_by_sql'

So you can see we access table "sites" but use columns of table "tags"

Vittly commented 10 years ago

The problem is about deep stack with long paths like:

/opt/ruby-ent/lib/ruby/gems/1.8/gems/slim_scrooge-1.0.12/lib/slim_scrooge/slim_scrooge.rb:31:in `find_with_callsite_key'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/slim_scrooge-1.0.12/lib/slim_scrooge/slim_scrooge.rb:17:in `find_by_sql'
app/helpers/widgets_helper.rb:4:in `render_widgets'
app/helpers/widgets_helper.rb:8:in `render_widgets'
app/views/layouts/ru.rhtml:15:in `_run_rhtml_app47views47layouts47cubic46rhtml'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/agent/method_tracer.rb:518:in `render'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/agent/method_tracer.rb:268:in `trace_execution_scoped'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/agent/method_tracer.rb:513:in `render'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/agent/instrumentation/controller_instrumentation.rb:320:in `perform_action'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/rack/agent_hooks.rb:22:in `call'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/rack/agent_hooks.rb:22:in `call'
/opt/ruby-ent/lib/ruby/gems/1.8/gems/newrelic_rpm-3.6.6.147/lib/new_relic/rack/browser_monitoring.rb:16:in `call'
config/initializers/middlewares/set_cookie_domain.rb:12:in `call'
config/initializers/flash_session_cookie.rb:25:in `call'
passenger (4.0.7) lib/phusion_passenger/rack/thread_handler_extension.rb:77:in `process_request'
passenger (4.0.7) lib/phusion_passenger/request_handler/thread_handler.rb:140:in `accept_and_process_next_request'
passenger (4.0.7) lib/phusion_passenger/request_handler/thread_handler.rb:108:in `main_loop'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:441:in `start_threads'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:435:in `initialize'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:435:in `new'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:435:in `start_threads'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:434:in `times'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:434:in `start_threads'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:433:in `synchronize'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:433:in `start_threads'
passenger (4.0.7) lib/phusion_passenger/request_handler.rb:200:in `main_loop'
passenger (4.0.7) helper-scripts/classic-rails-preloader.rb:159

Our project is big and may be there are too many places (call_sites in terms of slimscrooge) where collisions happen. I have slightly rewrite c-logic in slimscrooge to use in call_site calculations short tails of paths (scripnames and its first folders) and limit stack deep to 10 unique locations. With these new faster verion of slimscrooge code there are no such errors any more.

Is it a good idea to add my changes to slimscrooge project?

sdsykes commented 10 years ago

Yes, please do

Vittly commented 10 years ago

I didn't do this before. I should fork the project... Do I?

sdsykes commented 10 years ago

Hi... now you have commit rights here, so you can fork it or add straight here.

Vittly commented 10 years ago

I have commit my changes. If it is good we can update the gem Or not?

sdsykes commented 10 years ago

Yes I can fire off an updated gem shortly.

Vittly commented 10 years ago

Thank you, I'll be waiting for new gem