Skip to content

Loading…

Store the offline manifest entries in the cache. #276

Closed
wants to merge 3 commits into from

6 participants

@mpessas

We make the compress command store the entries of the manifest.json file in cache and the compress templatetag try to read the correct value from the cache instead of the manifest.json file.

Apostolos Be... added some commits
Apostolos Bessas Make compress command store manifest in cache.
The compress command will store the manifest in cache, so that reading
it afterwards from the storage is not needed.  This is especially
useful, when the manifest is not stored locally (e.g., when using S3).

We save each entry in the cache separately, instead of creating one
entry for the whole manifest. This way, the next time there are changes,
the compress command will keep old entries in the cache, so that
running processes are not affected. We rely on cache expiration to
remove those old entries.

Of course, we keep the old method as a fallback, in case a key is
missing from the cache. In practice, though, this should not happen at
all.
907ca5d
Apostolos Bessas Add tests for caching the manifest in OFFLINE mode. 9825f90
Apostolos Bessas Update docs for saving the offline manifest entries in the cache. 8681ce3
@jaap3

Thing is, get_offline_manifest already caches the parsed JSON in a module level variable for the duration of the python process. So there seems to be no merit in storing the individual values into a cache, which expires every 5 minutes by default.

@mpessas

There are two reasons for this:
a) The manifest is cached in-process. So, whenever a process starts it has to fetch the file. This can take quite some time, depending on what storage you use. Given that a process might get restarted sometimes (e.g. due to memory leaks), you pay a huge penalty whenever this happens. Additionally, during deployment, you usually restart all processes, which means that you have to pay that penalty for each one of them. This adds latency to the necessary time to respond to requests.
b) If you restart the processes after a certain number of handled requests, it could be the case that the process fetches a newer manifest.json file than the one it should have (i.e., the codebase has not been updated yet), which results in errors.

Regarding the expiration time, I am pretty sure no one uses the default 5min timeout :) And most django installations that care for such things usually deploy many times during the "timeout period" (we might deploy more than once a day).
That said, for small installations this change might make things worse (the process now has to read values from the cache instead of from a global variable). But any big web application could benefit from this.

@jaap3

Can't you just "warm up" your processes by calling get_offline_manifest in your wsgi script? This frontloads the overhead you seem so worried about.

@mpessas

Not really. That would mean you would have to restart your master process, whenever you update your servers, dropping any existing connections in the queue, instead of restarting the child processes.
Even so, you still have to fetch that file, just less times.
In large installations, using the cache is more convenient.

@travisbot

This pull request passes (merged 8681ce3 into 3723a67).

@alanjds

+1 to cache manifest. For now, I am wrapping {% compress %} tags into {% cache %} tags, but it does not invalidate automatically on 'compress' offline call. Use-case: Heroku dynos

@diox

Closing this because it is super-old and I'm really not a fan of adding more layers of complexity to offline caching anyway.

@diox diox closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 4, 2012
  1. Make compress command store manifest in cache.

    Apostolos Bessas committed
    The compress command will store the manifest in cache, so that reading
    it afterwards from the storage is not needed.  This is especially
    useful, when the manifest is not stored locally (e.g., when using S3).
    
    We save each entry in the cache separately, instead of creating one
    entry for the whole manifest. This way, the next time there are changes,
    the compress command will keep old entries in the cache, so that
    running processes are not affected. We rely on cache expiration to
    remove those old entries.
    
    Of course, we keep the old method as a fallback, in case a key is
    missing from the cache. In practice, though, this should not happen at
    all.
  2. Add tests for caching the manifest in OFFLINE mode.

    Apostolos Bessas committed
This page is out of date. Refresh to see the latest.
Showing with 89 additions and 17 deletions.
  1. +31 −0 compressor/cache.py
  2. +2 −9 compressor/templatetags/compress.py
  3. +44 −1 compressor/tests/test_offline.py
  4. +4 −3 docs/behind-the-scenes.txt
  5. +8 −4 docs/usage.txt
View
31 compressor/cache.py
@@ -13,6 +13,7 @@
from compressor.conf import settings
from compressor.storage import default_storage
from compressor.utils import get_mod_func
+from compressor.exceptions import OfflineGenerationError
_cachekey_func = None
@@ -62,6 +63,35 @@ def get_offline_manifest_filename():
return os.path.join(output_dir, settings.COMPRESS_OFFLINE_MANIFEST)
+def _get_manifest_cache_key(s):
+ return get_cachekey('manifest.%s' % s)
+
+
+def get_offline_value(key):
+ """Get the link corresponding to the specified key in OFFLINE mode."""
+ value = cache.get(_get_manifest_cache_key(key))
+ if value is not None:
+ return value
+
+ # Key is not in cache, we have to read it from manifest itself.
+ offline_manifest = get_offline_manifest()
+ if key in offline_manifest:
+ cache.set(_get_manifest_cache_key(key), offline_manifest[key])
+ return offline_manifest[key]
+ else:
+ raise OfflineGenerationError('You have offline compression '
+ 'enabled but key "%s" is missing from offline manifest. '
+ 'You may need to run "python manage.py compress".' % key)
+
+
+def _cache_manifest(manifest):
+ """Store the offline manifest to local cache."""
+ cached_manifest = {}
+ for k,v in manifest.items():
+ cache_key = _get_manifest_cache_key(k)
+ cached_manifest[cache_key] = v
+ cache.set_many(cached_manifest)
+
_offline_manifest = None
@@ -86,6 +116,7 @@ def write_offline_manifest(manifest):
default_storage.save(filename,
ContentFile(simplejson.dumps(manifest, indent=2)))
flush_offline_manifest()
+ _cache_manifest(manifest)
def get_templatetag_cachekey(compressor, mode, kind):
View
11 compressor/templatetags/compress.py
@@ -2,9 +2,8 @@
from django.core.exceptions import ImproperlyConfigured
from compressor.cache import (cache_get, cache_set, get_offline_hexdigest,
- get_offline_manifest, get_templatetag_cachekey)
+ get_offline_value, get_templatetag_cachekey)
from compressor.conf import settings
-from compressor.exceptions import OfflineGenerationError
from compressor.utils import get_class
register = template.Library()
@@ -63,13 +62,7 @@ def render_offline(self, context, forced):
"""
if self.is_offline_compression_enabled(forced) and not forced:
key = get_offline_hexdigest(self.nodelist.render(context))
- offline_manifest = get_offline_manifest()
- if key in offline_manifest:
- return offline_manifest[key]
- else:
- raise OfflineGenerationError('You have offline compression '
- 'enabled but key "%s" is missing from offline manifest. '
- 'You may need to run "python manage.py compress".' % key)
+ return get_offline_value(key)
def render_cached(self, compressor, kind, mode, forced=False):
"""
View
45 compressor/tests/test_offline.py
@@ -2,13 +2,16 @@
import os
from StringIO import StringIO
from unittest2 import skipIf
+from mock import patch
import django
from django.template import Template, Context
from django.test import TestCase
from django.core.management.base import CommandError
-from compressor.cache import flush_offline_manifest, get_offline_manifest
+from compressor.cache import (flush_offline_manifest, get_offline_manifest,
+ cache, _cache_manifest, _get_manifest_cache_key,
+ write_offline_manifest)
from compressor.conf import settings
from compressor.exceptions import OfflineGenerationError
from compressor.management.commands.compress import Command as CompressCommand
@@ -41,6 +44,7 @@ def setUp(self):
self.template_path = os.path.join(settings.TEMPLATE_DIRS[0], self.template_name)
self.template_file = open(self.template_path)
self.template = Template(self.template_file.read().decode(settings.FILE_CHARSET))
+ cache.clear()
def tearDown(self):
settings.COMPRESS_ENABLED = self._old_compress
@@ -236,3 +240,42 @@ def test_get_loaders(self):
self.assertTrue(isinstance(loaders[1], AppDirectoriesLoader))
finally:
settings.TEMPLATE_LOADERS = old_loaders
+
+
+class ManifestCacheTestCase(OfflineTestCaseMixin, TestCase):
+ """Tests for caching the offline manifest."""
+
+ templates_dir = "basic"
+ expected_hash = "f5e179b8eca4"
+
+ def test_cache_generation(self):
+ """Test saving the manifest to cache."""
+ manifest = {'1': 'a', '2': 'b'}
+ _cache_manifest(manifest)
+ for k,v in manifest.items():
+ self.assertEqual(cache.get(_get_manifest_cache_key(k)), v)
+
+ def test_compress_caches_manifest(self):
+ """Test that the compress command caches the generated manifest."""
+ manifest = {'1': 'a', '2': 'b'}
+ with patch('compressor.cache._cache_manifest') as m:
+ write_offline_manifest(manifest)
+ m.assert_called_with(manifest)
+
+ def test_key_fetched_from_manifest_when_not_in_cache(self):
+ """
+ Test fetching a key that is not in cache will try to bring it from
+ the manifest file.
+ """
+ count, result = CompressCommand().compress(
+ log=self.log, verbosity=self.verbosity
+ )
+ cache.clear()
+ rendered_template = self.template.render(
+ Context(settings.COMPRESS_OFFLINE_CONTEXT)
+ )
+ self.assertEqual(rendered_template, "".join(result) + "\n")
+ manifest = get_offline_manifest()
+ for key in manifest:
+ actual_key = _get_manifest_cache_key(key)
+ self.assertIsNotNone(cache.get(actual_key))
View
7 docs/behind-the-scenes.txt
@@ -10,13 +10,14 @@ Offline cache
-------------
If offline cache is activated, the first thing {% compress %} tries to do is
-retrieve the compressed version for its nodelist from the offline manifest
-cache. It doesn't parse, doesn't check the modified times of the files, doesn't
+retrieve the compressed version for its nodelist from the cache and, in case it
+is not found there, from the offline manifest cache.
+It doesn't parse, doesn't check the modified times of the files, doesn't
even know which files are concerned actually, since it doesn't look inside the
nodelist of the template block enclosed by the ``compress`` template tag.
The offline cache manifest is just a json file, stored on disk inside the
directory that holds the compressed files. The format of the manifest is simply
-a key <=> value dictionnary, with the hash of the nodelist being the key,
+a key <=> value dictionary, with the hash of the nodelist being the key,
and the HTML containing the element code for the combined file or piece of code
being the value. Generating the offline manifest, using the ``compress``
management command, also generates the combined files referenced in the manifest.
View
12 docs/usage.txt
@@ -148,10 +148,14 @@ If not specified, the ``COMPRESS_OFFLINE_CONTEXT`` will by default contain
the commonly used setting to refer to saved files ``MEDIA_URL`` and
``STATIC_URL`` (if specified in the settings).
-The result of running the ``compress`` management command will be cached
-in a file called ``manifest.json`` using the :attr:`configured storage
-<django.conf.settings.COMPRESS_STORAGE>` to be able to be transfered from your developement
-computer to the server easily.
+The result of running the ``compress`` management command will be saved
+in the cache as well as stored in a file called ``manifest.json``
+using the :attr:`configured storage <django.conf.settings.COMPRESS_STORAGE>`.
+The cache is used to avoid any delays from fetching and reading the file the first time it is accessed
+(especially, if it is not stored locally).
+The manifest file, on the other hand, is used as a fallback, in case a key in the cache has expired.
+Generating the manifest file has the added benefit of making it possible to compress the static files
+in a development computer and later transfer just the manifest file to the production servers.
.. _TEMPLATE_LOADERS: http://docs.djangoproject.com/en/dev/ref/settings/#template-loaders
Something went wrong with that request. Please try again.