> On April 14, 2015, 7:08 a.m., Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java, 
> > line 137
> > <https://reviews.apache.org/r/33098/diff/1/?file=924174#file924174line137>
> >
> >     this should be expireAfterAccess, right?
> >     
> >     maybe make the expiration configurable?

The problem with expireAfterAccess is that it make every access to the cache to 
update the cache. Since we're checking the cache for each entry written/read, 
that could be 10s of thousand times per sec. The impact of using 
expireAfterAccess is clearly visible (in terms of CPU usage and mem 
allocations) in the profiler.

We could make it configurable, though I'm not clear on how to define this 
configurable time in user perspective. Ideally, we shouldn't need to use a 
"cache", we should clean the map when a ledger is deleted, but the garbage 
collector is only notifying the LedgerStorage of the deletions. 

The simple point is that we just want to avoid leaking memory when ledger are 
created/deleted over the time.


> On April 14, 2015, 7:08 a.m., Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java, 
> > line 1097
> > <https://reviews.apache.org/r/33098/diff/1/?file=924174#file924174line1097>
> >
> >     it would be better to check if e.getCause instanceof IOException.

Ok, will change that.


> On April 14, 2015, 7:08 a.m., Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactoryImpl.java,
> >  line 41
> > <https://reviews.apache.org/r/33098/diff/1/?file=924175#file924175line41>
> >
> >     expireAfterAccess? configurable time?

Same as above comment


- Matteo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33098/#review79994
-----------------------------------------------------------


On April 11, 2015, 12:06 a.m., Matteo Merli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33098/
> -----------------------------------------------------------
> 
> (Updated April 11, 2015, 12:06 a.m.)
> 
> 
> Review request for bookkeeper.
> 
> 
> Bugs: BOOKKEEPER-852
>     https://issues.apache.org/jira/browse/BOOKKEEPER-852
> 
> 
> Repository: bookkeeper-git
> 
> 
> Description
> -------
> 
> Maps with ledger descriptors and master-keys are not cleaned after a ledger 
> gets deleted.
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 
> 8b40853 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactoryImpl.java
>  45be763 
> 
> Diff: https://reviews.apache.org/r/33098/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matteo Merli
> 
>

Reply via email to