You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Marijn Speelman <ma...@hyves.nl> on 2008/11/06 18:26:04 UTC

BasicHttpCache, use of unsynchronized LinkedHashMap

Hi,

The default BasicHttpCache is using an unsychronized LinkedHashMap which
can cause very strange problems when it has multiple threads talking to
it. We've been using this class in combination with a memcached backend
(when the http response is not in the in-memory cache we retrieve it
from our main memcached pool).

The problem that appears after a while is that the max capacity of the
LRU cache is not being respected, eventually resulting in an OOM error
after a day or two (with about 88,000 cached objects while our max
capacity was set to 10,000).

I know it says 'Basic' but I expected it to be simple, not 'unsuited for
production' :) For now I just made the three functions (add,get,remove)
synchronized, which of course results in a performance penalty but as it
seems, not a significant one (especially compared to a server leaking
memory ;)).

The reason why we used the BasicHttpCache and for instance not ehcache
is that it was just what we needed (a really really simple in-memory
cache), and we're still using an older shindig version of around May
that didn't have ehcache in it yet.

I suggest making the BasicHttpCache thread safe, or at least adding a
comment to warn people that it's not suited for production for this
reason (but then again, making it thread safe but slower is probably
fine for non-production as well). Or was I just silly for using this in
a production environment? :) I find it quite hard to tell what exactly
is production ready in Shindig, and what is just a sample implementation
you should never really use. Is there some documentation on this
(configure this, create a better class instead of class X that supports
this and that)?

Marijn

Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Ian Boston <ie...@tfd.co.uk>.
Happy to pick this one up, as I was guilty of putting ehcache there  
in the first place.

probably just needs, the following, and moving some properties into  
the central properties file.

I wont do anything unless you say so, just incase you have other plans.

Ian

diff --git a/java/common/src/main/java/org/apache/shindig/common/ 
cache/CacheProvider.java b/java/common/src/main/java/org/apache/ 
shindig/common/cache/CacheProvider.java
index a5fd167..fac41d3 100644
--- a/java/common/src/main/java/org/apache/shindig/common/cache/ 
CacheProvider.java
+++ b/java/common/src/main/java/org/apache/shindig/common/cache/ 
CacheProvider.java
@@ -19,10 +19,12 @@ package org.apache.shindig.common.cache;

  import com.google.inject.ImplementedBy;

+import org.apache.shindig.common.cache.ehcache.EhCacheCacheProvider;
+
  /**
   *
   */
-@ImplementedBy(DefaultCacheProvider.class)
+@ImplementedBy(EhCacheCacheProvider.class)
  public interface CacheProvider {
    /**
     * Create a named single instance cache in this cache manager, if  
the cache


On 7 Nov 2008, at 02:47, Kevin Brown wrote:

> On Thu, Nov 6, 2008 at 12:33 PM, Paul Lindner <pl...@hi5.com>  
> wrote:
> I can attest that the ehcache implementation holds up in production  
> usage.
>
> Indeed. It seems to me that there's no reason not to pull this in.  
> I'll get a patch going now.
>
>
>
> On Nov 6, 2008, at 12:23 PM, Kevin Brown wrote:
>
> Actually, we should probably just get rid of the LRU cache here  
> (the real
> culprit) and make everything use EH cache.  I don't see any strong  
> reason
> not to -- the cache provider can be changed to simply *always* use  
> named
> instances, which certainly simplifies things.
>
> Ian, you wrote the ehcache implementation. Do you have any strong  
> opinions
> here?
>
> On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:
>
> By convention, Basic* are not considered production worthy. For the  
> HTTP
> cache, I'd never use a pure in memory solution, and through ehcache
> configuration you should be able to wire something more robust fairly
> easily.
>
> We can probably make this class thread safe anyway though. If we're  
> really
> worried about performance, a ReadWriteLock would probably do the  
> trick.
>
>
> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl>  
> wrote:
>
> Hi,
>
> The default BasicHttpCache is using an unsychronized LinkedHashMap  
> which
> can cause very strange problems when it has multiple threads  
> talking to
> it. We've been using this class in combination with a memcached  
> backend
> (when the http response is not in the in-memory cache we retrieve it
> from our main memcached pool).
>
> The problem that appears after a while is that the max capacity of the
> LRU cache is not being respected, eventually resulting in an OOM error
> after a day or two (with about 88,000 cached objects while our max
> capacity was set to 10,000).
>
> I know it says 'Basic' but I expected it to be simple, not  
> 'unsuited for
> production' :) For now I just made the three functions  
> (add,get,remove)
> synchronized, which of course results in a performance penalty but  
> as it
> seems, not a significant one (especially compared to a server leaking
> memory ;)).
>
> The reason why we used the BasicHttpCache and for instance not ehcache
> is that it was just what we needed (a really really simple in-memory
> cache), and we're still using an older shindig version of around May
> that didn't have ehcache in it yet.
>
> I suggest making the BasicHttpCache thread safe, or at least adding a
> comment to warn people that it's not suited for production for this
> reason (but then again, making it thread safe but slower is probably
> fine for non-production as well). Or was I just silly for using  
> this in
> a production environment? :) I find it quite hard to tell what exactly
> is production ready in Shindig, and what is just a sample  
> implementation
> you should never really use. Is there some documentation on this
> (configure this, create a better class instead of class X that  
> supports
> this and that)?
>
> Marijn
>
>
>
>
> Paul Lindner
> plindner@hi5.com
>
>
>
>


Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Brian Eaton <be...@google.com>.
On Fri, Nov 7, 2008 at 2:02 AM, Marijn Speelman <ma...@hyves.nl> wrote:
> For example, we've been using the BasicHttpFetcher,
> BasicGadget/MessageBundleFactory in production and it has been running
> quite smoothly, with very little changes in those areas and no
> performance problems, apart from the BasicHttpCache.

You need to wire up a proxy to prevent bad people from using
BasicHttpFetcher to poke and prod at your internal network.

Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Kevin Brown <et...@google.com>.
Hey everyone,

I've checked in changes here that should make us reasonably able to say that
we're providing a production-worthy caching solution now.

Please note that there are substantial configuration changes, since this
must now be done through ehcache configuration, but I have wired default
values that I think will be suitable for most uses.

I've made HttpResponse implement Externalizable so that it can be safely
used with EhCache's remote and disk-based caches. EhCache provides 4 remote
cache integrations out of the box and also provides facilities for extending
it, such as using Memcache (there are a few projects that provide an EhCache
binding to memcached).

I'd appreciate it if people with more experience than I have using ehcache
(that would be roughly zero :)) and / or java serialization could take a
look at my patch and provide some feedback.

Unfortunately, I won't be online so much next week due to a family
emergency, but I think this completes one of two major remaining
shortcomings with the out of the box gadget server. The other is our poor
BasicHttpFetcher. Now that we've shed some other dependencies, I think we
might be able to safely upgrade this to an HttpClient 4.0 based solution.
Paul Lindner had a great piece of code that did just that a few months back.

Paul, do you think it would be possible to wire this into Shindig core as a
replacement for BasicHttpFetcher? The only minor issue I recall from your
old implementation (other than the compatibility problem) was that it didn't
provide a way to specify a proxy server, but otherwise it was great.

On Fri, Nov 7, 2008 at 2:02 AM, Marijn Speelman <ma...@hyves.nl> wrote:

> Ok, thanks, then we'll probably start using ehcache, although the
> BasicHttpCache is now working fine as first layer in front of our
> memcached pool now.
>
> Is there a good list somewhere for containers to know what exactly you
> should change in Shindig to make it production worthy? Without any live
> OS experience it's hard to think about all the use cases and
> bottlenecks, so some notes from the developers can be useful, security,
> performance and feature wise.
>
> For example, we've been using the BasicHttpFetcher,
> BasicGadget/MessageBundleFactory in production and it has been running
> quite smoothly, with very little changes in those areas and no
> performance problems, apart from the BasicHttpCache.
>
> Kevin Brown wrote:
> > On Thu, Nov 6, 2008 at 12:33 PM, Paul Lindner <pl...@hi5.com> wrote:
> >
> >> I can attest that the ehcache implementation holds up in production
> usage.
> >
> >
> > Indeed. It seems to me that there's no reason not to pull this in. I'll
> get
> > a patch going now.
> >
> >
> >>
> >> On Nov 6, 2008, at 12:23 PM, Kevin Brown wrote:
> >>
> >>  Actually, we should probably just get rid of the LRU cache here (the
> real
> >>> culprit) and make everything use EH cache.  I don't see any strong
> reason
> >>> not to -- the cache provider can be changed to simply *always* use
> named
> >>> instances, which certainly simplifies things.
> >>>
> >>> Ian, you wrote the ehcache implementation. Do you have any strong
> opinions
> >>> here?
> >>>
> >>> On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:
> >>>
> >>>  By convention, Basic* are not considered production worthy. For the
> HTTP
> >>>> cache, I'd never use a pure in memory solution, and through ehcache
> >>>> configuration you should be able to wire something more robust fairly
> >>>> easily.
> >>>>
> >>>> We can probably make this class thread safe anyway though. If we're
> >>>> really
> >>>> worried about performance, a ReadWriteLock would probably do the
> trick.
> >>>>
> >>>>
> >>>> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl>
> wrote:
> >>>>
> >>>>  Hi,
> >>>>> The default BasicHttpCache is using an unsychronized LinkedHashMap
> which
> >>>>> can cause very strange problems when it has multiple threads talking
> to
> >>>>> it. We've been using this class in combination with a memcached
> backend
> >>>>> (when the http response is not in the in-memory cache we retrieve it
> >>>>> from our main memcached pool).
> >>>>>
> >>>>> The problem that appears after a while is that the max capacity of
> the
> >>>>> LRU cache is not being respected, eventually resulting in an OOM
> error
> >>>>> after a day or two (with about 88,000 cached objects while our max
> >>>>> capacity was set to 10,000).
> >>>>>
> >>>>> I know it says 'Basic' but I expected it to be simple, not 'unsuited
> for
> >>>>> production' :) For now I just made the three functions
> (add,get,remove)
> >>>>> synchronized, which of course results in a performance penalty but as
> it
> >>>>> seems, not a significant one (especially compared to a server leaking
> >>>>> memory ;)).
> >>>>>
> >>>>> The reason why we used the BasicHttpCache and for instance not
> ehcache
> >>>>> is that it was just what we needed (a really really simple in-memory
> >>>>> cache), and we're still using an older shindig version of around May
> >>>>> that didn't have ehcache in it yet.
> >>>>>
> >>>>> I suggest making the BasicHttpCache thread safe, or at least adding a
> >>>>> comment to warn people that it's not suited for production for this
> >>>>> reason (but then again, making it thread safe but slower is probably
> >>>>> fine for non-production as well). Or was I just silly for using this
> in
> >>>>> a production environment? :) I find it quite hard to tell what
> exactly
> >>>>> is production ready in Shindig, and what is just a sample
> implementation
> >>>>> you should never really use. Is there some documentation on this
> >>>>> (configure this, create a better class instead of class X that
> supports
> >>>>> this and that)?
> >>>>>
> >>>>> Marijn
> >>>>>
> >>>>>
> >>>>
> >> Paul Lindner
> >> plindner@hi5.com
> >>
> >>
> >>
> >>
> >
>
>

Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Marijn Speelman <ma...@hyves.nl>.
Ok, thanks, then we'll probably start using ehcache, although the
BasicHttpCache is now working fine as first layer in front of our
memcached pool now.

Is there a good list somewhere for containers to know what exactly you
should change in Shindig to make it production worthy? Without any live
OS experience it's hard to think about all the use cases and
bottlenecks, so some notes from the developers can be useful, security,
performance and feature wise.

For example, we've been using the BasicHttpFetcher,
BasicGadget/MessageBundleFactory in production and it has been running
quite smoothly, with very little changes in those areas and no
performance problems, apart from the BasicHttpCache.

Kevin Brown wrote:
> On Thu, Nov 6, 2008 at 12:33 PM, Paul Lindner <pl...@hi5.com> wrote:
> 
>> I can attest that the ehcache implementation holds up in production usage.
> 
> 
> Indeed. It seems to me that there's no reason not to pull this in. I'll get
> a patch going now.
> 
> 
>>
>> On Nov 6, 2008, at 12:23 PM, Kevin Brown wrote:
>>
>>  Actually, we should probably just get rid of the LRU cache here (the real
>>> culprit) and make everything use EH cache.  I don't see any strong reason
>>> not to -- the cache provider can be changed to simply *always* use named
>>> instances, which certainly simplifies things.
>>>
>>> Ian, you wrote the ehcache implementation. Do you have any strong opinions
>>> here?
>>>
>>> On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:
>>>
>>>  By convention, Basic* are not considered production worthy. For the HTTP
>>>> cache, I'd never use a pure in memory solution, and through ehcache
>>>> configuration you should be able to wire something more robust fairly
>>>> easily.
>>>>
>>>> We can probably make this class thread safe anyway though. If we're
>>>> really
>>>> worried about performance, a ReadWriteLock would probably do the trick.
>>>>
>>>>
>>>> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl> wrote:
>>>>
>>>>  Hi,
>>>>> The default BasicHttpCache is using an unsychronized LinkedHashMap which
>>>>> can cause very strange problems when it has multiple threads talking to
>>>>> it. We've been using this class in combination with a memcached backend
>>>>> (when the http response is not in the in-memory cache we retrieve it
>>>>> from our main memcached pool).
>>>>>
>>>>> The problem that appears after a while is that the max capacity of the
>>>>> LRU cache is not being respected, eventually resulting in an OOM error
>>>>> after a day or two (with about 88,000 cached objects while our max
>>>>> capacity was set to 10,000).
>>>>>
>>>>> I know it says 'Basic' but I expected it to be simple, not 'unsuited for
>>>>> production' :) For now I just made the three functions (add,get,remove)
>>>>> synchronized, which of course results in a performance penalty but as it
>>>>> seems, not a significant one (especially compared to a server leaking
>>>>> memory ;)).
>>>>>
>>>>> The reason why we used the BasicHttpCache and for instance not ehcache
>>>>> is that it was just what we needed (a really really simple in-memory
>>>>> cache), and we're still using an older shindig version of around May
>>>>> that didn't have ehcache in it yet.
>>>>>
>>>>> I suggest making the BasicHttpCache thread safe, or at least adding a
>>>>> comment to warn people that it's not suited for production for this
>>>>> reason (but then again, making it thread safe but slower is probably
>>>>> fine for non-production as well). Or was I just silly for using this in
>>>>> a production environment? :) I find it quite hard to tell what exactly
>>>>> is production ready in Shindig, and what is just a sample implementation
>>>>> you should never really use. Is there some documentation on this
>>>>> (configure this, create a better class instead of class X that supports
>>>>> this and that)?
>>>>>
>>>>> Marijn
>>>>>
>>>>>
>>>>
>> Paul Lindner
>> plindner@hi5.com
>>
>>
>>
>>
> 


Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Kevin Brown <et...@google.com>.
On Thu, Nov 6, 2008 at 12:33 PM, Paul Lindner <pl...@hi5.com> wrote:

> I can attest that the ehcache implementation holds up in production usage.


Indeed. It seems to me that there's no reason not to pull this in. I'll get
a patch going now.


>
>
> On Nov 6, 2008, at 12:23 PM, Kevin Brown wrote:
>
>  Actually, we should probably just get rid of the LRU cache here (the real
>> culprit) and make everything use EH cache.  I don't see any strong reason
>> not to -- the cache provider can be changed to simply *always* use named
>> instances, which certainly simplifies things.
>>
>> Ian, you wrote the ehcache implementation. Do you have any strong opinions
>> here?
>>
>> On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:
>>
>>  By convention, Basic* are not considered production worthy. For the HTTP
>>> cache, I'd never use a pure in memory solution, and through ehcache
>>> configuration you should be able to wire something more robust fairly
>>> easily.
>>>
>>> We can probably make this class thread safe anyway though. If we're
>>> really
>>> worried about performance, a ReadWriteLock would probably do the trick.
>>>
>>>
>>> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl> wrote:
>>>
>>>  Hi,
>>>>
>>>> The default BasicHttpCache is using an unsychronized LinkedHashMap which
>>>> can cause very strange problems when it has multiple threads talking to
>>>> it. We've been using this class in combination with a memcached backend
>>>> (when the http response is not in the in-memory cache we retrieve it
>>>> from our main memcached pool).
>>>>
>>>> The problem that appears after a while is that the max capacity of the
>>>> LRU cache is not being respected, eventually resulting in an OOM error
>>>> after a day or two (with about 88,000 cached objects while our max
>>>> capacity was set to 10,000).
>>>>
>>>> I know it says 'Basic' but I expected it to be simple, not 'unsuited for
>>>> production' :) For now I just made the three functions (add,get,remove)
>>>> synchronized, which of course results in a performance penalty but as it
>>>> seems, not a significant one (especially compared to a server leaking
>>>> memory ;)).
>>>>
>>>> The reason why we used the BasicHttpCache and for instance not ehcache
>>>> is that it was just what we needed (a really really simple in-memory
>>>> cache), and we're still using an older shindig version of around May
>>>> that didn't have ehcache in it yet.
>>>>
>>>> I suggest making the BasicHttpCache thread safe, or at least adding a
>>>> comment to warn people that it's not suited for production for this
>>>> reason (but then again, making it thread safe but slower is probably
>>>> fine for non-production as well). Or was I just silly for using this in
>>>> a production environment? :) I find it quite hard to tell what exactly
>>>> is production ready in Shindig, and what is just a sample implementation
>>>> you should never really use. Is there some documentation on this
>>>> (configure this, create a better class instead of class X that supports
>>>> this and that)?
>>>>
>>>> Marijn
>>>>
>>>>
>>>
>>>
> Paul Lindner
> plindner@hi5.com
>
>
>
>

Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Paul Lindner <pl...@hi5.com>.
I can attest that the ehcache implementation holds up in production  
usage.

On Nov 6, 2008, at 12:23 PM, Kevin Brown wrote:

> Actually, we should probably just get rid of the LRU cache here (the  
> real
> culprit) and make everything use EH cache.  I don't see any strong  
> reason
> not to -- the cache provider can be changed to simply *always* use  
> named
> instances, which certainly simplifies things.
>
> Ian, you wrote the ehcache implementation. Do you have any strong  
> opinions
> here?
>
> On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:
>
>> By convention, Basic* are not considered production worthy. For the  
>> HTTP
>> cache, I'd never use a pure in memory solution, and through ehcache
>> configuration you should be able to wire something more robust fairly
>> easily.
>>
>> We can probably make this class thread safe anyway though. If we're  
>> really
>> worried about performance, a ReadWriteLock would probably do the  
>> trick.
>>
>>
>> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl>  
>> wrote:
>>
>>> Hi,
>>>
>>> The default BasicHttpCache is using an unsychronized LinkedHashMap  
>>> which
>>> can cause very strange problems when it has multiple threads  
>>> talking to
>>> it. We've been using this class in combination with a memcached  
>>> backend
>>> (when the http response is not in the in-memory cache we retrieve it
>>> from our main memcached pool).
>>>
>>> The problem that appears after a while is that the max capacity of  
>>> the
>>> LRU cache is not being respected, eventually resulting in an OOM  
>>> error
>>> after a day or two (with about 88,000 cached objects while our max
>>> capacity was set to 10,000).
>>>
>>> I know it says 'Basic' but I expected it to be simple, not  
>>> 'unsuited for
>>> production' :) For now I just made the three functions  
>>> (add,get,remove)
>>> synchronized, which of course results in a performance penalty but  
>>> as it
>>> seems, not a significant one (especially compared to a server  
>>> leaking
>>> memory ;)).
>>>
>>> The reason why we used the BasicHttpCache and for instance not  
>>> ehcache
>>> is that it was just what we needed (a really really simple in-memory
>>> cache), and we're still using an older shindig version of around May
>>> that didn't have ehcache in it yet.
>>>
>>> I suggest making the BasicHttpCache thread safe, or at least  
>>> adding a
>>> comment to warn people that it's not suited for production for this
>>> reason (but then again, making it thread safe but slower is probably
>>> fine for non-production as well). Or was I just silly for using  
>>> this in
>>> a production environment? :) I find it quite hard to tell what  
>>> exactly
>>> is production ready in Shindig, and what is just a sample  
>>> implementation
>>> you should never really use. Is there some documentation on this
>>> (configure this, create a better class instead of class X that  
>>> supports
>>> this and that)?
>>>
>>> Marijn
>>>
>>
>>

Paul Lindner
plindner@hi5.com




Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Kevin Brown <et...@google.com>.
Actually, we should probably just get rid of the LRU cache here (the real
culprit) and make everything use EH cache.  I don't see any strong reason
not to -- the cache provider can be changed to simply *always* use named
instances, which certainly simplifies things.

Ian, you wrote the ehcache implementation. Do you have any strong opinions
here?

On Thu, Nov 6, 2008 at 9:51 AM, Kevin Brown <et...@google.com> wrote:

> By convention, Basic* are not considered production worthy. For the HTTP
> cache, I'd never use a pure in memory solution, and through ehcache
> configuration you should be able to wire something more robust fairly
> easily.
>
> We can probably make this class thread safe anyway though. If we're really
> worried about performance, a ReadWriteLock would probably do the trick.
>
>
> On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl> wrote:
>
>> Hi,
>>
>> The default BasicHttpCache is using an unsychronized LinkedHashMap which
>> can cause very strange problems when it has multiple threads talking to
>> it. We've been using this class in combination with a memcached backend
>> (when the http response is not in the in-memory cache we retrieve it
>> from our main memcached pool).
>>
>> The problem that appears after a while is that the max capacity of the
>> LRU cache is not being respected, eventually resulting in an OOM error
>> after a day or two (with about 88,000 cached objects while our max
>> capacity was set to 10,000).
>>
>> I know it says 'Basic' but I expected it to be simple, not 'unsuited for
>> production' :) For now I just made the three functions (add,get,remove)
>> synchronized, which of course results in a performance penalty but as it
>> seems, not a significant one (especially compared to a server leaking
>> memory ;)).
>>
>> The reason why we used the BasicHttpCache and for instance not ehcache
>> is that it was just what we needed (a really really simple in-memory
>> cache), and we're still using an older shindig version of around May
>> that didn't have ehcache in it yet.
>>
>> I suggest making the BasicHttpCache thread safe, or at least adding a
>> comment to warn people that it's not suited for production for this
>> reason (but then again, making it thread safe but slower is probably
>> fine for non-production as well). Or was I just silly for using this in
>> a production environment? :) I find it quite hard to tell what exactly
>> is production ready in Shindig, and what is just a sample implementation
>> you should never really use. Is there some documentation on this
>> (configure this, create a better class instead of class X that supports
>> this and that)?
>>
>> Marijn
>>
>
>

Re: BasicHttpCache, use of unsynchronized LinkedHashMap

Posted by Kevin Brown <et...@google.com>.
By convention, Basic* are not considered production worthy. For the HTTP
cache, I'd never use a pure in memory solution, and through ehcache
configuration you should be able to wire something more robust fairly
easily.

We can probably make this class thread safe anyway though. If we're really
worried about performance, a ReadWriteLock would probably do the trick.

On Thu, Nov 6, 2008 at 9:26 AM, Marijn Speelman <ma...@hyves.nl> wrote:

> Hi,
>
> The default BasicHttpCache is using an unsychronized LinkedHashMap which
> can cause very strange problems when it has multiple threads talking to
> it. We've been using this class in combination with a memcached backend
> (when the http response is not in the in-memory cache we retrieve it
> from our main memcached pool).
>
> The problem that appears after a while is that the max capacity of the
> LRU cache is not being respected, eventually resulting in an OOM error
> after a day or two (with about 88,000 cached objects while our max
> capacity was set to 10,000).
>
> I know it says 'Basic' but I expected it to be simple, not 'unsuited for
> production' :) For now I just made the three functions (add,get,remove)
> synchronized, which of course results in a performance penalty but as it
> seems, not a significant one (especially compared to a server leaking
> memory ;)).
>
> The reason why we used the BasicHttpCache and for instance not ehcache
> is that it was just what we needed (a really really simple in-memory
> cache), and we're still using an older shindig version of around May
> that didn't have ehcache in it yet.
>
> I suggest making the BasicHttpCache thread safe, or at least adding a
> comment to warn people that it's not suited for production for this
> reason (but then again, making it thread safe but slower is probably
> fine for non-production as well). Or was I just silly for using this in
> a production environment? :) I find it quite hard to tell what exactly
> is production ready in Shindig, and what is just a sample implementation
> you should never really use. Is there some documentation on this
> (configure this, create a better class instead of class X that supports
> this and that)?
>
> Marijn
>