incubator-blur-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron McCurry <amccu...@gmail.com>
Subject Re: SlabAllocationCacheValueBufferPool thread-safe?
Date Tue, 23 Aug 2016 15:48:20 GMT
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 <
ravikumar.govindarajan@gmail.com> 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 <amccurry@gmail.com> 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 <
> > ravikumar.govindarajan@gmail.com> 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 <amccurry@gmail.com>
> > wrote:
> > >
> > > > Ok. Thanks!  I will patch the code and run some tests.
> > > >
> > > > On Monday, August 1, 2016, Ravikumar Govindarajan <
> > > > ravikumar.govindarajan@gmail.com> 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 <amccurry@gmail.com
> > > > > <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 <
> > > > > > ravikumar.govindarajan@gmail.com <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
<
> > > > > > > ravikumar.govindarajan@gmail.com <javascript:;>>
wrote:
> > > > > > >
> > > > > > > > Thanks for the feedback Aaron
> > > > > > > >
> > > > > > > > On Thu, Jul 21, 2016 at 6:42 PM, Aaron McCurry <
> > > amccurry@gmail.com
> > > > > <javascript:;>>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >> On Thu, Jul 21, 2016 at 1:55 AM, Ravikumar Govindarajan
<
> > > > > > > >> ravikumar.govindarajan@gmail.com <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
<
> > > > > amccurry@gmail.com <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
> <
> > > > > > > >> > > ravikumar.govindarajan@gmail.com <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?
> > > > > > > >> > > >
> > > > > > > >> > >
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message