This project is archived and is in readonly mode.

#1283 ✓resolved
sds

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

  • sds

    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.

  • Pratik

    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

    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...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages