Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default cache settings are weird #4125

Closed
benjaoming opened this issue Jul 20, 2015 · 2 comments
Closed

Default cache settings are weird #4125

benjaoming opened this issue Jul 20, 2015 · 2 comments

Comments

@benjaoming
Copy link
Contributor

This block of code in kalite.settings.base is very hard to figure out:

if CACHE_TIME != 0:  # None can mean infinite caching to some functions
    # When we change versions, cache changes, too
    KEY_PREFIX = ".".join(version.VERSION)

    # File-based cache
    install_location_hash = hashlib.sha1(".".join(version.VERSION)).hexdigest()
    username = getpass.getuser() or "unknown_user"
    cache_dir_name = "kalite_web_cache_%s" % (username)
    CACHE_LOCATION = os.path.realpath(getattr(local_settings, "CACHE_LOCATION", os.path.join(tempfile.gettempdir(), cache_dir_name, install_location_hash))) + "/"
    CACHES["file_based_cache"] = {
        'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
        'LOCATION': CACHE_LOCATION,  # this is kind of OS-specific, so dangerous.
        'TIMEOUT': CACHE_TIME,  # should be consistent
        'OPTIONS': {
            'MAX_ENTRIES': getattr(local_settings, "CACHE_MAX_ENTRIES", 5*2000)  # 2000 entries=~10,000 files
        },
    }

    # Memory-based cache
    CACHES["mem_cache"] = {
        'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
        'LOCATION': 'unique-snowflake',
        'TIMEOUT': CACHE_TIME, # should be consistent
        'OPTIONS': {
            'MAX_ENTRIES': getattr(local_settings, "CACHE_MAX_ENTRIES", 5*2000)  # 2000 entries=~10,000 files
        },
    }

    # The chosen cache
    CACHE_NAME = getattr(local_settings, "CACHE_NAME", "file_based_cache")

What does it do?

  1. Creates a file-based cache with a 5 year time out.
  2. Stores the cache in a temporary location!? (So much for the 5 years)
  3. Initializes settings for "mem_cache" which are NEVER used anywhere.
  4. Then finally, it sets CACHE_NAME, used in fle_utils.internet.webcache (yes, we have a package called internet). So what's CACHES['default'] supposed to be for!?
  5. At every start of KA Lite kalite start the cache is explicitly deleted anyways? Why do we bother to have a cache? Why do we bother with the tempdir? What's with the 5 years?

There are no really good conventions for caching, but it seems that issues about caching would historically have to do with memory limitations on some systems.

I would suggest just opting for a persistent, disk-based default cache and switch to a memory-based cache in kalite.project.settings.dev.

@benjaoming benjaoming self-assigned this Jul 20, 2015
@benjaoming benjaoming added this to the 0.14.0 milestone Jul 20, 2015
This was referenced Jul 20, 2015
rtibbles added a commit that referenced this issue Jul 29, 2015
@aronasorman
Copy link
Collaborator

Though it's a huge code smell, I don't think this is a release blocker. Moving to 0.15.

@benjaoming
Copy link
Contributor Author

There's no serious issues remaining except making an overall design decision on caching. IMO most of the code smell is gone.

########################
# Storage and caching
########################

# Local memory cache is to expensive to use for the page cache.
#   instead, use a file-based cache.
# By default, cache for maximum possible time.
#   Note: caching for 100 years can be too large a value
#   sys.maxint also can be too large (causes ValueError), since it's added to the current time.
#   Caching for the lesser of (100 years) or (5 years less than the max int) should work.
_5_years = 5 * 365 * 24 * 60 * 60
_100_years = 100 * 365 * 24 * 60 * 60
_max_cache_time = min(_100_years, sys.maxint - time.time() - _5_years)
CACHE_TIME = getattr(local_settings, "CACHE_TIME", _max_cache_time)

# Sessions use the default cache, and we want a local memory cache for that.
CACHE_LOCATION = os.path.realpath(getattr(
    local_settings,
    "CACHE_LOCATION",
    os.path.join(
        USER_DATA_ROOT,
        'cache',
    )
))

CACHES = {
    "default": {
        'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
        'LOCATION': CACHE_LOCATION,  # this is kind of OS-specific, so dangerous.
        'TIMEOUT': CACHE_TIME,  # should be consistent
        'OPTIONS': {
            'MAX_ENTRIES': getattr(local_settings, "CACHE_MAX_ENTRIES", 5 * 2000)  # 2000 entries=~10,000 files
        },
    }
}

# Prefix the cache with the version string so we don't experience problems with
# updates
KEY_PREFIX = version.VERSION

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants