hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Moore, Jonathan" <Jonathan_Mo...@Comcast.com>
Subject RE: [HttpCache][PATCH] Caching API review
Date Mon, 12 Jul 2010 19:04:25 GMT
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:rjefts@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-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message