hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mac fang <mac.had...@gmail.com>
Subject Re: Looks like duplicate in MemoryStoreFlusher flushSomeRegions()
Date Wed, 26 Jan 2011 04:34:28 GMT
Oh, yes, I checked the code, the method protected CompactionRequest
addToRegionsInQueue(HRegion r, int p) {

contains the:

      if (queuedRequest == null ||
          newRequest.getPriority() < queuedRequest.getPriority()) {
        LOG.trace("Inserting region in queue. " + newRequest);
        regionsInQueue.put(r, newRequest);

So it should be OK. Thanks for remind.

regards
macf


On Wed, Jan 26, 2011 at 10:30 AM, Ryan Rawson <ryanobjc@gmail.com> wrote:

> it does seem like the call to compactionRequested calls in
>
> (Class MemStoreFlusher)
>  private synchronized void flushSomeRegions();
>
> are superfluous.  Due to the nature of the compaction algorithm this
> should not cause extra compaction, so the impact should be minimal,
> but ideally should be removed.
>
> -ryan
>
> On Tue, Jan 25, 2011 at 6:23 PM, Ryan Rawson <ryanobjc@gmail.com> wrote:
> > the call to compactionRequested() only puts the region on a queue to
> > be compacted, so if there is unintended duplication, it wont actually
> > hold anything up.
> >
> > -ryan
> >
> > On Tue, Jan 25, 2011 at 6:05 PM, mac fang <mac.hadoop@gmail.com> wrote:
> >> Guys, since the flushCache will make the write/read suspend. I am NOT
> sure
> >> if it is necessary here.
> >>
> >> On Mon, Jan 24, 2011 at 1:48 PM, mac fang <mac.hadoop@gmail.com> wrote:
> >>
> >>> Yes, I mean the server.compactSplitThread.compactionRequested(region,
> >>> getName());
> >>>
> >>> in flushRegion, it will do the
> server.compactSplitThread.compactionRequested(region,
> >>> getName());
> >>>
> >>> *seems we don't need to do it again in the following logic (can you
> guys
> >>> see the lines in bold, **!flushRegion(biggestMemStoreRegion, true) and
> *
> >>> *
> >>> *
> >>> *    for (HRegion region : regionsToCompact) {
> >>>       server.compactSplitThread.compactionRequested(region, getName());
> >>>     }*
> >>> *
> >>> *
> >>> *
> >>> *
> >>> *regards*
> >>> macf
> >>>
> >>>
> >>>     if (*!flushRegion(biggestMemStoreRegion, true)*) {
> >>>         LOG.warn("Flush failed");
> >>>         break;
> >>>       }
> >>>       regionsToCompact.add(biggestMemStoreRegion);
> >>>     }
> >>>     for (HRegion region : regionsToCompact) {
> >>>       *server.compactSplitThread.compactionRequested(region,
> getName());*
> >>>     }
> >>>
> >>> > > in flushRegion
> >>> > >
> >>> > >  private boolean flushRegion(final HRegion region, final boolean
> >>> > > emergencyFlush) {
> >>> > >    synchronized (this.regionsInQueue) {
> >>> > >      FlushQueueEntry fqe = this.regionsInQueue.remove(region);
> >>> > >      if (fqe != null && emergencyFlush) {
> >>> > >        // Need to remove from region from delay queue.  When NOT
an
> >>> > >        // emergencyFlush, then item was removed via a
> flushQueue.poll.
> >>> > >        flushQueue.remove(fqe);
> >>> > >      }
> >>> > >      lock.lock();
> >>> > >    }
> >>> > >    try {
> >>> > >      if (region.flushcache()) {
> >>> > >        *server.compactSplitThread.compactionRequested(region,
> >>> getName());*
> >>> > >      }
> >>>
> >>> On Mon, Jan 24, 2011 at 6:40 AM, Ted Yu <yuzhihong@gmail.com> wrote:
> >>>
> >>>> I think he was referring to this line:
> >>>>
> >>>> server.compactSplitThread.compactionRequested(region, getName());
> >>>>
> >>>> On Sun, Jan 23, 2011 at 10:52 AM, Stack <stack@duboce.net> wrote:
> >>>>
> >>>> > Hello Mac Fang:  Which lines in the below?  Your colorizing didn't
> >>>> > come across in the mail.  Thanks, St.Ack
> >>>> >
> >>>> > On Sun, Jan 23, 2011 at 6:23 AM, mac fang <mac.hadoop@gmail.com>
> wrote:
> >>>> > > Hi, guys,
> >>>> > >
> >>>> > > see the below codes in* MemStoreFlusher.java*, i am not sure
if
> those
> >>>> >  lines
> >>>> > > in orange are the same and looks like they are trying to do
the
> same
> >>>> > logic.
> >>>> > > Are they redundant?
> >>>> > >
> >>>> > > regards
> >>>> > > macf
> >>>> > >
> >>>> > >    if (!flushRegion(biggestMemStoreRegion, true)) {
> >>>> > >        LOG.warn("Flush failed");
> >>>> > >        break;
> >>>> > >      }
> >>>> > >      regionsToCompact.add(biggestMemStoreRegion);
> >>>> > >    }
> >>>> > >    for (HRegion region : regionsToCompact) {
> >>>> > >      server.compactSplitThread.compactionRequested(region,
> getName());
> >>>> > >    }
> >>>> > >
> >>>> > > in flushRegion
> >>>> > >
> >>>> > >  private boolean flushRegion(final HRegion region, final boolean
> >>>> > > emergencyFlush) {
> >>>> > >    synchronized (this.regionsInQueue) {
> >>>> > >      FlushQueueEntry fqe = this.regionsInQueue.remove(region);
> >>>> > >      if (fqe != null && emergencyFlush) {
> >>>> > >        // Need to remove from region from delay queue.  When
NOT
> an
> >>>> > >        // emergencyFlush, then item was removed via a
> flushQueue.poll.
> >>>> > >        flushQueue.remove(fqe);
> >>>> > >      }
> >>>> > >      lock.lock();
> >>>> > >    }
> >>>> > >    try {
> >>>> > >      if (region.flushcache()) {
> >>>> > >        server.compactSplitThread.compactionRequested(region,
> >>>> getName());
> >>>> > >      }
> >>>> > >
> >>>> >
> >>>>
> >>>
> >>>
> >>
> >
>

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