PF the small patch we made for this. We had a specific requirement to solve. Don't know how useful it will be for the community!
On Thu, Aug 25, 2016 at 11:40 AM, Ravikumar Govindarajan < [email protected]> wrote: > Sure, will share the patch in a couple of days... > > On Tue, Aug 23, 2016 at 9:18 PM, Aaron McCurry <[email protected]> wrote: > >> Cool, sounds good. If you could send me the changes you have made that >> would be great. It would make it easier to integrate your changes back >> into the project. Thanks! >> >> Aaron >> >> On Wed, Aug 17, 2016 at 8:40 AM, Ravikumar Govindarajan < >> [email protected]> wrote: >> >> > I removed _cacheValueQuietRefCannotBeReleased & instead directly used a >> > ByteArrayCacheValue every-time fillQuietly() is called. >> > >> > Now searches seem to work correctly. Not sure if it's because of >> clone() or >> > may be something else... >> > >> > FYI, I modified ByteArrayCacheValue to use a Store-Buffer to go easy on >> gc >> > >> > On Wed, Aug 10, 2016 at 8:12 PM, Aaron McCurry <[email protected]> >> wrote: >> > >> > > I'm not sure why that IOE is happening but if you want to change the >> > quiet >> > > behavior this is where you can control it. There's a config and an >> > object >> > > there to change the behavior. >> > > >> > > https://github.com/apache/incubator-blur/blob/master/ >> > > blur-store/src/main/java/org/apache/blur/store/ >> > > BlockCacheDirectoryFactoryV2.java#L171 >> > > >> > > On Wed, Aug 10, 2016 at 10:37 AM, Ravikumar Govindarajan < >> > > [email protected]> wrote: >> > > >> > > > Aaron, >> > > > >> > > > Just one more help... >> > > > >> > > > I hardcoded _quiet=true in CacheIndexInput.java and started the >> > > > shard-server. It seems to mangle the cached-bytes & results in >> > > IOException >> > > > during searches. Merges however work smoothly... >> > > > >> > > > I do know that _quiet is meant only for merge. But there is a >> use-case >> > I >> > > am >> > > > working on, which will need this setting during searches also... >> > > > >> > > > Any quick suggestions for this issue? >> > > > >> > > > -- >> > > > Ravi >> > > > >> > > > On Thu, Aug 4, 2016 at 2:43 PM, Aaron McCurry <[email protected]> >> > > wrote: >> > > > >> > > > > Ok. Thanks! I will patch the code and run some tests. >> > > > > >> > > > > On Monday, August 1, 2016, Ravikumar Govindarajan < >> > > > > [email protected]> wrote: >> > > > > >> > > > > > We did a simple expiry check. Works fine as of now... >> > > > > > >> > > > > > private CacheValue lookup(boolean quietly) { >> > > > > > >> > > > > > CacheValue cacheValue = _indexInputCache.get(_key. >> > getBlockId()); >> > > > > > >> > > > > > if(cacheValue == null || *cacheValue.isExpired()*) { >> > > > > > >> > > > > > .... >> > > > > > >> > > > > > } >> > > > > > >> > > > > > } >> > > > > > >> > > > > > On Sun, Jul 31, 2016 at 2:14 AM, Aaron McCurry < >> [email protected] >> > > > > > <javascript:;>> wrote: >> > > > > > >> > > > > > > Ok I have to look into it. Do you have a patch available? >> > > > > > > >> > > > > > > On Thu, Jul 28, 2016 at 4:47 AM, Ravikumar Govindarajan < >> > > > > > > [email protected] <javascript:;>> wrote: >> > > > > > > >> > > > > > > > One issue we found in CacheIndexInput.java which is causing >> NPE >> > > > > > > > >> > > > > > > > private CacheValue lookup(boolean quietly) { >> > > > > > > > >> > > > > > > > CacheValue cacheValue = _indexInputCache.get(_key. >> > > > getBlockId()); >> > > > > > > > >> > > > > > > > ....... >> > > > > > > > >> > > > > > > > return cacheValue; >> > > > > > > > >> > > > > > > > //There is no eviction check for the CacheValue >> returned >> > > from >> > > > > > > > IndexInputCache, causing NPE >> > > > > > > > >> > > > > > > > } >> > > > > > > > >> > > > > > > > Also, lookup method blindly adds to _indexInputCache before >> > > > > returning. >> > > > > > > > Instead, it would be better if it is done inside the >> null-check >> > > > > loop... >> > > > > > > > >> > > > > > > > On Fri, Jul 22, 2016 at 6:09 PM, Ravikumar Govindarajan < >> > > > > > > > [email protected] <javascript:;>> wrote: >> > > > > > > > >> > > > > > > > > Thanks for the feedback Aaron >> > > > > > > > > >> > > > > > > > > On Thu, Jul 21, 2016 at 6:42 PM, Aaron McCurry < >> > > > [email protected] >> > > > > > <javascript:;>> >> > > > > > > > wrote: >> > > > > > > > > >> > > > > > > > >> On Thu, Jul 21, 2016 at 1:55 AM, Ravikumar Govindarajan < >> > > > > > > > >> [email protected] <javascript:;>> wrote: >> > > > > > > > >> >> > > > > > > > >> > Just now saw BlockLocks code. It is documented to be >> > > > > thread-safe. >> > > > > > > > >> Apologize >> > > > > > > > >> > for the trouble... >> > > > > > > > >> > >> > > > > > > > >> > Btw, a small nit. The below method is not returning >> true. >> > Is >> > > > > that >> > > > > > > > >> > intentional? >> > > > > > > > >> > >> > > > > > > > >> > boolean releaseIfValid(long address) { >> > > > > > > > >> > >> > > > > > > > >> > if (address >= _address && address < >> _maxAddress) { >> > > > > > > > >> > >> > > > > > > > >> > long offset = address - _address; >> > > > > > > > >> > >> > > > > > > > >> > int index = (int) (offset / _chunkSize); >> > > > > > > > >> > >> > > > > > > > >> > _locks.clear(index); >> > > > > > > > >> > >> > > > > > > > >> > } >> > > > > > > > >> > >> > > > > > > > >> > return false; >> > > > > > > > >> > >> > > > > > > > >> > } >> > > > > > > > >> > >> > > > > > > > >> >> > > > > > > > >> In my 30 second review I think you are right. It should >> > > > probably >> > > > > > > return >> > > > > > > > >> true. However I want to alanyze what happens with the >> > current >> > > > > code >> > > > > > > so I >> > > > > > > > >> can write a test that proves there is a problem (because >> > there >> > > > > > > probably >> > > > > > > > >> is) >> > > > > > > > >> and fix it. >> > > > > > > > >> >> > > > > > > > >> >> > > > > > > > >> > >> > > > > > > > >> > Also, I thought a background thread can attempt merging >> > > > sparsely >> > > > > > > > >> populated >> > > > > > > > >> > slabs into one single slab & release free-mem (in 128MB >> > > > chunks) >> > > > > > back >> > > > > > > > to >> > > > > > > > >> > OS... >> > > > > > > > >> > >> > > > > > > > >> >> > > > > > > > >> I think this is a good idea, I just didn't get to writing >> > it. >> > > > > > > > >> >> > > > > > > > >> >> > > > > > > > >> > >> > > > > > > > >> > You think it could be beneficial or it would make it >> > > > needlessly >> > > > > > > > complex? >> > > > > > > > >> > >> > > > > > > > >> >> > > > > > > > >> I think for dedicated servers is might be overkill, but >> for >> > a >> > > > > mixed >> > > > > > > > >> workload environment (think docker containers and the >> like) >> > it >> > > > > would >> > > > > > > be >> > > > > > > > >> useful. >> > > > > > > > >> >> > > > > > > > >> Aaron >> > > > > > > > >> >> > > > > > > > >> >> > > > > > > > >> > >> > > > > > > > >> > On Wed, Jul 20, 2016 at 11:38 PM, Aaron McCurry < >> > > > > > [email protected] <javascript:;> >> > > > > > > > >> > > > > > > > >> > wrote: >> > > > > > > > >> > >> > > > > > > > >> > > I don't think there is a race condition because the >> > > > allocation >> > > > > > > > occurs >> > > > > > > > >> > > atomically in the BlockLocks class. Do see a >> problem? >> > > Let >> > > > me >> > > > > > > know. >> > > > > > > > >> > > >> > > > > > > > >> > > Aaron >> > > > > > > > >> > > >> > > > > > > > >> > > On Wed, Jul 20, 2016 at 9:19 AM, Ravikumar >> Govindarajan >> > < >> > > > > > > > >> > > [email protected] <javascript:;>> >> wrote: >> > > > > > > > >> > > >> > > > > > > > >> > > > I came across the following in >> > > > > > > > >> SlabAllocationCacheValueBufferPool.java. >> > > > > > > > >> > > Is >> > > > > > > > >> > > > the below method thread-safe? >> > > > > > > > >> > > > >> > > > > > > > >> > > > @Override >> > > > > > > > >> > > > >> > > > > > > > >> > > > public CacheValue getCacheValue(int >> cacheBlockSize) >> > { >> > > > > > > > >> > > > >> > > > > > > > >> > > > validCacheBlockSize(cacheBlockSize); >> > > > > > > > >> > > > >> > > > > > > > >> > > > int numberOfChunks = getNumberOfChunks( >> > > > cacheBlockSize); >> > > > > > > > >> > > > >> > > > > > > > >> > > > ... >> > > > > > > > >> > > > >> > > > > > > > >> > > > } >> > > > > > > > >> > > > >> > > > > > > > >> > > > >> > > > > > > > >> > > > It does allocation in a tight-loop using >> BlockLocks, >> > > Slab >> > > > & >> > > > > > > > Chunks. >> > > > > > > > >> Is >> > > > > > > > >> > > > there a race-condition where 2 threads can pick >> same >> > > slab >> > > > & >> > > > > > > chunk? >> > > > > > > > >> > > > >> > > > > > > > >> > > >> > > > > > > > >> > >> > > > > > > > >> >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
