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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: >>> >>>> I think he was referring to this line: >>>> >>>> server.compactSplitThread.compactionRequested(region, getName()); >>>> >>>> On Sun, Jan 23, 2011 at 10:52 AM, Stack <[email protected]> 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 <[email protected]> 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()); >>>> > > } >>>> > > >>>> > >>>> >>> >>> >> >
