#1283 Storing query cache in Thread.current breaks when multiple DBs are used - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1283 ✓resolved

Storing query cache in Thread.current breaks when multiple DBs are used

Reported by sds | October 28th, 2008 @ 08:38 AM

We use rails with multiple databases. Some models come from one database, and some from another. We use establish_connection to make sure the models are connected correctly. (Using the use_db plugin.)

Consider the following code from my unit tests:


def test_database
  User.cache do
    assert_equal "database1_test", User.connection.current_database, "db must be correct"
    assert_equal "database2_test", Product.connection.current_database, "db must be correct"
  end
end

This fails in 2.2 because the query cache is not stored on a per connection basis.

User.cache sets Thread.current['query_cache_enabled'] to true for the block.

Then because the query cache is stored in Thread.current['query_cache'] and because the two queries are the same (SELECT DATABASE() as db) the second call to connection.current_database returns the wrong result.

Perhaps this could be fixed by adding the database name to the key used, like this in active_record/connection_adapters/abstract/query_cache.rb:


      def query_cache
        Thread.current["query_cache_#{@config[:database]}"] ||= {}
      end

      def query_cache=(cache)
        Thread.current["query_cache_#{@config[:database]}"] = cache
      end

With this patch tests pass (and my real code now works).

This isn't quite perfect though - we have to add the ||= assignment to query_cache because calling User.cache only initialises the hash for the db associated with the User model.

Anyone have a better idea?

Comments and changes to this ticket

You flagged this item as spam.
sds October 28th, 2008 @ 09:14 AM
- Title changed from “Storing query cache in Thread.current breaks when multiple DBs are used” to “[patch] Storing query cache in Thread.current breaks when multiple DBs are used”
- Tag changed from “2.2.rc1, activerecord, cache, query” to “2.2.rc1, activerecord, cache, patch, query”
I attach a patch that fixes this along the lines of the above, but slightly improved.
- query_cache.patch 944 Bytes
Pratik October 29th, 2008 @ 09:46 PM
- Milestone cleared.
- Title changed from “[patch] Storing query cache in Thread.current breaks when multiple DBs are used” to “Storing query cache in Thread.current breaks when multiple DBs are used”
- Tag changed from “2.2.rc1, activerecord, cache, patch, query” to “activerecord, cache, patch, query”
- Assigned user set to “josh”
I think the correct fix is to move query cache to connection object.

@Josh : Wanna take a stab at this ?
Repository October 30th, 2008 @ 08:48 PM
- State changed from “new” to “resolved”
(from [0c84b6f9eda20c30b66d8fb99fba637edc1bc37a]) Use database name in query cache thread local key [#1283 state:resolved] http://github.com/rails/rails/co...