Code review comment for lp:~leonardr/launchpad/test-representation-cache

Revision history for this message
Leonard Richardson (leonardr) wrote :

> A better approach would be for the trigger to notify another process
> of the keys that need invalidating, and have that process do it
> asynchronously. This would be less overhead than our existing Slony-I
> replication triggers. There would be some lag.
>
> An alternative, which I'd need to investigate, would be to piggy back
> on the existing Slony-I replication information. Whenever a row is
> updated or deleted, we generate events containing this information so
> the changes can be applied on the subscriber database nodes. There
> would be some lag here too, but no measurable database impact.

I'm +1 on either of these ideas. The only downside is that the invalidation algorithm will become more complex over time, as I make lazr.restful cache more things and as we create more versions of the Launchpad web service.

Here's what I mean. Right now when User(29,) is invalidated we need to remove the following three cache keys:

User(29,),application/json,beta
User(29,),application/json,1.0
User(29,),application/json,devel

That's one key for every web service version. We need to have access to a list of the currently active versions, either managed separately or obtained from getUtility(IWebServiceConfiguration).active_Versions.

You also brought up the fact that all our Launchpad instances share memcached hardware. So we need to change Launchpad's key generation to include the instance name. And we need to change the invalidation code to have a list of all instance names, and to invalidate these nine keys:

User(29,),application/json,beta,edge
User(29,),application/json,beta,staging
User(29,),application/json,beta,production
User(29,),application/json,1.0,edge
User(29,),application/json,1.0,staging
User(29,),application/json,1.0,production
User(29,),application/json,devel,edge
User(29,),application/json,devel,staging
User(29,),application/json,devel,production

In the future we may change lazr.restful to cache the WADL and/or HTML representations as well as the JSON representation. In that case there could be up to 27 cache keys for User(29,).

This isn't unmanageable, but it would be much simpler if we could tell memcached to delete "User(29,).*"

> I'm wondering if it is possible that the cache gets populated, but the
> webservice request fails, causing an invalid value to get stored. If
> we don't populate the cache on updates then it should be fine.

I'm pretty sure the cache is populated right before the representation is returned to the client. If there's an error it will happen during the representation creation. If for some reason an error happens afterwards it doesn't change the fact that the representation is accurate. The cache is certainly not populated on updates. So I think we're OK.

« Back to merge proposal