Set Redis max memory policy

[annehaley]

Resolves #398

This PR changes the command for the Redis container in development to the following:

["redis-server", "--maxmemory", "4096MB", "--maxmemory-policy", "allkeys-lru"]

For production, we can't set the max memory size of the Heroku Redis add-on without upgrading the plan (we currently use the mini plan), but we can set the max memory policy to allkeys-lru.

[brianhelba]

Ideally we'd set this in CI too, but GH Actions doesn't let you pass a custom command to a service.

[annehaley]

Yeah, that's fine. It wouldn't make a difference for our tests anyway.

[brianhelba]

I have concerns about how this will affect the use of Django Channels, which uses Redis to mediate websocket connections.

I think a better policy may be volatile-lru, but that might require cooperation from large_image. I'm happy to help investigate more.

[manthey]

I have concerns about how this will affect the use of Django Channels, which uses Redis to mediate websocket connections.

I think a better policy may be volatile-lru, but that might require cooperation from large_image. I'm happy to help investigate more.

volatile-lru requires that entries have a TTL. large_image uses redis (or other tile caches) strictly as LRU -- there is no reason to evict tiles except for the memory area being full and the then LRU us a sensible policy. If websocket connections are more recently used than the speed at which tiles are cached, this won't functionally be a problem, but fundamentally, these two sets of data have very different concerns. The correct architecture is really have two separate caches, one that is LRU for tiles and one that is ALWAYS as large as total possible websocket connections .

[brianhelba]

#363 set our cache implementation to use large_image...RedisCache, which doesn't support setting TTL on keys.

However, we could:

• Switch back to using django_large_image...DjangoCache
  • Just set LARGE_IMAGE_CACHE_BACKEND = "django" or unset it entirely as it's the django_large_image default
• DjangoCache will rely on Django's general cache framework
• Configure Django's cache framework to use Redis
  • Set a TIMEOUT to add by default to all keys
  • Use a more specific cache name, rather than making a "default" Django cache, in case other parts of Django want to use the "default" cache
    • You should be able to set LARGE_IMAGE_CACHE_NAME accordingly

[brianhelba]

I think volatile-lru gives us the balance we want. We can set the TTL value on the tile cache to be arbitrarily long, so tiles will practically only get evicted by memory pressure, not time. In this case, the TTL is still essential as a marker that the tile data doesn't need to be preserved for the application to function correctly.

[manthey]

I think volatile-lru gives us the balance we want. We can set the TTL value on the tile cache to be arbitrarily long, so tiles will practically only get evicted by memory pressure, not time. In this case, the TTL is still essential as a marker that the tile data doesn't need to be preserved for the application to function correctly.

I can't tell from how the redis docs are written if the eviction is anything that is past its TTL or anything that has a TTL at all. If the former, then the shortest possible TTL would permit tile to be evicted (but redis would normally keep them since it takes effort to evict things). If the latter, the a long TTL would prevent needless eviction.