Created JIRA issue. Please do go through it. Not sure if community will find this useful https://issues.apache.org/jira/browse/BLUR-481
On Mon, Sep 5, 2016 at 9:50 PM, Aaron McCurry <[email protected]> wrote: > Hey Ravi. Not sure if you attached the patch on email or not. The mail > system removes attachments. You can open an issue here: > > https://issues.apache.org/jira/browse/BLUR > > Or post it to a gist or if it really is small you can just include in an > email. > > Thanks! > > Aaron > > On Tue, Aug 30, 2016 at 1:44 AM, Ravikumar Govindarajan < > [email protected]> wrote: > > > 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? > >>> > > > > > > > >> > > > > >>> > > > > > > > >> > > > >>> > > > > > > > >> > > >>> > > > > > > > >> > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >> > >> > > >
