Cache review request counts for quick lookup in the dashboard.

Review Request #1848 — Created Oct. 19, 2010 and submitted

Information

Review Board
master

Reviewers

The Dashboard is one of our most expensive yet most frequently visited pages, largely due to the number of expensive queries happening on each load. Every page load would have at least 6 queries for counts on the sidebar, possibly more depending on watched groups and such. As a database grows, this gets much slower.

We now cache the counts in the database. The Profile model now stores counts for starred review requests, outgoing pending review requests, all outgoing review requests, direct pending incoming review requests ("To Me"), and total pending incoming review requests. The Group model stores a count of incoming pending review requests.

These counts are updated when saving a ReviewRequest (specifically, when creating, publishing, closing, or reopening one) and when starring/unstarring review requests.

This reduces the query count on the dashboard by 7 queries.

It also converts the shipit_count field to use the new CounterField, since we're doing the same thing there, and consistency is good. Given that Counterfield is really just an IntegerField, this required no evolution.

There are unit tests that attempt to cover all the cases where we may end up updating these counts. It's possible that we may find bugs where the counts get out of sync. Fortunately, this would be nothing more than an annoyance on the sidebar, and we could provide a way to recompute the counts for users.
I've only been able to test this on my small SQLite setup, which is already not all that slow on the dashboard. However, I have verified that we do reduce by 7 queries, and we know that these can take longer on a significantly larger database.

The unit tests all pass.

All the hand testing I've done have paid off. I've created, starred, unstarred, discarded, submitted, and reopened review requests and saw the counts I expected.
david
  1. Most of this looks awesome. I'd like to hear your thoughts about how it will interact with the URL namespacing for LocalSite.
    1. Bother. I don't know.
      
      I imagine some profile things would need to be LocalSite-dependent anyway. Maybe we need a LocalSiteProfile or some such?
  2. reviewboard/reviews/models.py (Diff revision 1)
     
     
    Do we have to worried about SUBMITTED->DISCARDED or vice-versa?
    1. I don't think so. You can do this from code, but not from UI. Maybe I'll play with bullet-proofing that, but I'd rather get this in first.
  3. reviewboard/reviews/models.py (Diff revision 1)
     
     
    Is there any reason for this to not be at the top?
    1. Resolves a circular dependency. Profile imports this file for some models.
  4. 
      
chipx86
david
  1. 
      
  2. reviewboard/reviews/models.py (Diff revisions 1 - 2)
     
     
     
     
    Alignment?
  3. reviewboard/reviews/models.py (Diff revisions 1 - 2)
     
     
     
     
     
     
     
     
     
     
    These will decrement all of the matching ones, no? Don't we want this to be specific to this particular site_profile?
    1. Fixed these and added tests.
  4. reviewboard/reviews/models.py (Diff revisions 1 - 2)
     
     
     
     
     
     
     
     
     
     
    Same question here.
  5. 
      
chipx86
Review request changed
david
  1. Yay :)
  2. 
      
Loading...