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 <[email protected]> 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 <[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());
> >>>> > > }
> >>>> > >
> >>>> >
> >>>>
> >>>
> >>>
> >>
> >
>
