Hi Oleg, I'm not sure what you're trying to accomplish here. You're right that sometimes multiple entries need to get updated at the same time in a consistent fashion, which is why the callback is provided (and then the BasicHttpCache marks this as a synchronized method). It seems like your patch just adds a layer of explicit locking around what already exists.
It's also possible that the underlying cache mechanism does not support the level of atomicity required to support an explicit getLock() (for example, a memcached does not have a pessimistic lock, but it does have an atomic compare-and-swap operation, allowing for optimistic locking). The memcached implementation of #update would have to apply the compare-and-swap with a possible retry on contention. At the same time, I think you are raising a valid concern here, which is that the *interface* of HttpCacheUpdateCallback suggests that this is a one-for-one swap of a single cache entry, but in reality sometimes other things occur. The primary need for updating multiple records comes in the case of responses with the Vary header, where we are maintaining multiple variants as well as a "parent" entry that tracks them all (so that we can invalidate all variants of a resource when necessary). It's also probably worth noting, though, that we don't actually *update* more than one record at a time. The one place you changed in your patch was the case where we got a *new* variant (i.e. a complete response), which is just #put into the cache, completely overwriting anything previously there, and then we *update* the parent to track the new variantURI. Right now non-variants, variants, and parent entries are all stored in the same cache, and I think this is actually the crux of the problem; there's probably a better domain model for this than the one we've got, and I suspect that would make this problem go away. However, all things being equal, what we've got currently works, and I would be hesitant to change the cache API without another actual cache implementation to drive the API changes. Jon -----Original Message----- From: Richie Jefts [mailto:rje...@rfcode.com] Sent: Monday, July 12, 2010 12:37 PM To: dev@hc.apache.org Subject: Re: [HttpCache][PATCH] Caching API review I dislike getLock(). It exposes implementation details. Why not keep the old style of code and make it more explicit it may modify multiple entries. Something like. BasicHttpCache.update(String url, HttpCacheUpdate update); It ensures callers don't have to mess with the lock since it all happens behind the scenes for them. Plus it allows the locking of BasicHttpCache to change if necessary without callers having to deal with the change. richie On 07/12/2010 11:02 AM, Oleg Kalnichevski wrote: > Jon et al > > I started looking at the Caching API more closely primarily to find out > how difficult / feasible it should be to put together alternative cache > backends such as echache or file system based. I stumbled upon a few > minor issues I thought I should discuss with you. > > (1) HttpCacheUpdateCallback > > I think I understand the rationale behind the interface but kind of > dislike the fact that its implementations often manipulate the cache > behind the scene in non-obvious ways. One call to #updateCacheEntry may > result in mutation of multiple entries and multiple invocations of > #putEntry methods. Do we need #updateCacheEntry at all? What we need is > ability to update the cache while ensuring the consistency of its > content. Why not just use an exclusive lock to execute multiple cache > operations as a unit of work? > > Please take a look at the patch attached and let me know if you can live > with the proposed changes? > > (2) Do we really need HttpCacheEntrySerializer? It does not appear to be > used anywhere but in tests. > > Oleg > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
