search

alphagov / trade-tariff-backend

Enabling the population and distribution via API of UK Customs tariffs and duties
MIT License
7 stars 6 forks source link

Rabl caching #151

Closed matthewford closed 10 years ago

matthewford commented 10 years ago

This improves performance of the template generation of the JSON, would need to measure the exact gain and see if the memory consumption is worth the speed bump.

On my VM it cuts around half the render time.

jabley commented 10 years ago

Please hold. I have recollections of this being a problem in the past. Will have a dig.

matthewford commented 10 years ago

We decided against using the rabl cache, and to focus on just improving the SQL queries (https://github.com/alphagov/trade-tariff-backend/issues/2)

However after running under newrelic the majority of the time is spent rendering the json. Turning on the rabl cache helps, although this probably needs testing with some traffic as it could grow the RAM. We could use redis as the cache store.

matthewford commented 10 years ago

@bradleywright we have a solution to cache the generated api responses in this PR - although its an in memory cache it would be good to test this out before pushing it live.

bradwright commented 10 years ago

@matthewford great! Regarding in-memory caching, I'd rather now: we can install memcache on the boxes pretty easily, but it won't be clustered. Should improve performance significantly though, even without clustering.

matthewford commented 10 years ago

@bradleywright I remembered the issue that James brought up - it was to do with the cache not respecting the date. I have added that as part of the cache key so it should be fine.

I have chosen a rather arbitrary memory cache limit of 30 MB, some advice of it this is too much would be good.

jabley commented 10 years ago

I'd like TTL as well as constrained size. We should be monitoring cache hits and tune appropriately.

matthewford commented 10 years ago

@jabley I've set a global TTL to 1 day, but I'm not sure how to set a constrained size, do you mean per key or in total? the default max size is 1MB gziped (per key).

jabley commented 10 years ago

Who's doing the work to get memcache deployed?

bradwright commented 10 years ago

memcached is already deployed on all the backend machine, which is where tariff-api is hosted.

bradwright commented 10 years ago

So is this ready to merge? Travis doesn't seem happy.

matthewford commented 10 years ago

@bradleywright force pushed a rebase with master - it contained a fix for slow tests, which was what tarvis was complaining about (timeout)

bradwright commented 10 years ago

Let's see how it performs.