Created by Ricardo Kirkner on 2013-02-19 and last modified on 2013-02-19
Get this branch:
bzr branch lp:~ubuntuone-pqm-team/django-modeldict/stable
Members of Ubuntu One PQM Team can upload to this branch. Log in for directions.

Branch merges

Related bugs

Related blueprints

Branch information

Ubuntu One PQM Team

Recent revisions

68. By David Cramer on 2012-12-04


67. By Jeff Pollard <email address hidden> on 2012-11-27

Ship 1.4.0

66. By Jeff Pollard <email address hidden> on 2012-11-09

Attempt to appease the dependency gods

65. By Jeff Pollard <email address hidden> on 2012-11-09

Fix bug that caused us to pull down things every time

So there was a bug in modeldict that caused the entire switch set to be pulled from cache any time `_cleanup()` was called on the modeldict, which happened every time the `request_finished` and `task_postrun` signals were fired. This was bad, as it means we have a cache `get()` calls to pull down the heavy switch set every single page load and celery task.

The bug happened because `_cleanup()` reset the `_last_checked_for_remote_changes` variable to `None`, effectively clobbering any information that the modeldict had to know what it's own personal `last_modified` timestamp was. Thus, it had no way to know if the remote cache had changed given its version.

Practically, it caused this line of code to be run:

return int(cache_last_updated) > self._last_checked_for_remote_changes

`cache_last_updated` was the timestamp of the `last_updated` value in remote cache (when the switch set last changed), and `self._last_checked_for_remote_changes` was `None`, reset by `_cleanup()`. No matter what valid `cache_last_updated` value there is, comparing `any_value > None` is always `True`, so the modeldict thought that remote had changes and it pulled them down.

The change was to add a separate `_local_last_updated` instance variable that actually tracked the cached `last_updated` value. This leaves `_last_checked_for_remote_changes` to only be used to track when the modeldict last synced its local cache with the remote one, and to be reset without blowing away the locally cached `last_updated` value.

Test Plan: Existing tests, plus I added another test that asserts when cleanup is called it does not `get()` the whole switch set from the cache.

Reviewers: dcramer, ted

Reviewed By: dcramer

Differential Revision: http://phabricator.local.disqus.net/D4150

64. By Jeff Pollard <email address hidden> on 2012-11-09

Refactor base.py to be a little easier to follow.

My brain was exploding with all the terminology (cache, local_cache, _cache, etc), so I refactored the variable names and whatnot to be a little more semantic.

NOTE: this does **not** fix the cache hit performance regression, but this was a dependency on it to make the logic actually read and make sense.

Test Plan: existing

Reviewers: dcramer

Reviewed By: dcramer

Differential Revision: http://phabricator.local.disqus.net/D4132

63. By Jeff Pollard <email address hidden> on 2012-11-08

Add .arcconfig

62. By David Cramer on 2012-09-13


61. By David Cramer on 2012-08-21

Correct _cleanup comment

60. By David Cramer on 2012-08-21


59. By David Cramer on 2012-08-21


Branch metadata

Branch format:
Branch format 7
Repository format:
Bazaar repository format 2a (needs bzr 1.16 or later)
This branch contains Public information 
Everyone can see this information.