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? >>> > > > > > > > >> > > > >>> > > > > > > > >> > > >>> > > > > > > > >> > >>> > > > > > > > >> >>> > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >>> >> >> >
