Mailing-List: contact blur-user-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: blur-user@incubator.apache.org
MIME-Version: 1.0
In-Reply-To: <CAB6tTr3qKdeghk1KDTo1PpyN6Cnh4tDwVYskas7Kg35jUCgtEQ@mail.gmail.com>
References: <CAGW2whRZSTeAVgk8SY-RbFNMfH-8+XwyVHm1WpZCzrLm=OFhrQ@mail.gmail.com>
 <CAB6tTr15-DfcuSU6FPLhrmQ_=6YxuxgpZbL5c72URCpQ4=Gfyg@mail.gmail.com>
 <CAGW2whRYhppd+24YOs-wC4sCEXPExT2rfnatH7obzAZHDkKA-g@mail.gmail.com>
 <CAB6tTr3Shpo82jiHo_4eGOv7yvuc1VNC+Y6t3yfP27S1CdHzoQ@mail.gmail.com>
 <CAGW2whQc+vW5-9V8fkKhBeCc8QfCT=Uq40R6dwZq4_R8im94Kw@mail.gmail.com>
 <CAGW2whSLsJ6ca39UW_Ezw_SnTv_=Ek6iciK8SvRscGbusVKt9w@mail.gmail.com> <CAB6tTr3qKdeghk1KDTo1PpyN6Cnh4tDwVYskas7Kg35jUCgtEQ@mail.gmail.com>
From: Ravikumar Govindarajan <ravikumar.govindarajan@gmail.com>
Date: Mon, 1 Aug 2016 12:28:43 +0530
Message-ID: <CAGW2whRk_G2i8958zsSRt793R2KTnGxV_q5=3uwKdkb-9bznxg@mail.gmail.com>
Subject: Re: SlabAllocationCacheValueBufferPool thread-safe?
To: "blur-user@incubator.apache.org" <blur-user@incubator.apache.org>
Content-Type: multipart/alternative; boundary=001a114114ce8e44310538fd217b
archived-at: Mon, 01 Aug 2016 06:59:02 -0000

--001a114114ce8e44310538fd217b
Content-Type: text/plain; charset=UTF-8

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> 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> 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> wrote:
> >
> > > Thanks for the feedback Aaron
> > >
> > > On Thu, Jul 21, 2016 at 6:42 PM, Aaron McCurry <amccurry@gmail.com>
> > wrote:
> > >
> > >> On Thu, Jul 21, 2016 at 1:55 AM, Ravikumar Govindarajan <
> > >> ravikumar.govindarajan@gmail.com> 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
> >
> > >> > 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> 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?
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

--001a114114ce8e44310538fd217b--