hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Akash Ashok <thehellma...@gmail.com>
Subject Re: Inappropriate conditionLOG.isDebugEnabled() in HregionServer.java
Date Fri, 24 Jun 2011 14:36:31 GMT
Yes I agree that would have been an optimization if that condition would
ever fail. But its a condition that will always pass because its a local
variable that initialized to false and thus   if (!onlyMetaRegionsRemaining)
 always passes .

Please correct me if I am missing something here.

Thanks

On Fri, Jun 24, 2011 at 7:52 PM, Ted Yu <yuzhihong@gmail.com> wrote:

> I think the if condition below can be kept. It is an optimization because
> isOnlyMetaRegionsRemaining() would loop through
> this.onlineRegions.entrySet()
>
> On Fri, Jun 24, 2011 at 7:08 AM, Akash Ashok <thehellmaker@gmail.com>
> wrote:
>
> > Sure I shall file a Bug
> >
> > Also I dnt think this condition checking itself is required in run()
> >
> >  if (!onlyMetaRegionsRemaining) {
> >              onlyMetaRegionsRemaining = isOnlyMetaRegionsRemaining();
> >  }
> >
> >  isOnlyMetaRegionsRemaining() is anyways gonna initialize it right ?
> > without
> > checking the condition we can run
> >
> > onlyMetaRegionsRemaining = isOnlyMetaRegionsRemaining();
> >
> > Secondly as u said in  isOnlyMetaRegionsRemaining() could be initialized
> to
> > true instead of setting it to true everytime in the loop
> >
> > Thanks
> >
> > On Fri, Jun 24, 2011 at 6:37 PM, Ted Yu <yuzhihong@gmail.com> wrote:
> >
> > > Good catch.
> > > Also, I think in isOnlyMetaRegionsRemaining(), onlyMetaRegionsRemaining
> > > should be initialized to true so that the cleanup at line 638 can
> proceed
> > > if
> > > this.onlineRegions is empty.
> > >
> > > Mind filing a bug ?
> > >
> > > Thanks
> > >
> > > On Fri, Jun 24, 2011 at 5:52 AM, Akash Ashok <thehellmaker@gmail.com>
> > > wrote:
> > >
> > > > stop-hbase.sh never stopped once deployed the 0.91 trunk jar onto my
> > > hbase
> > > > setup. Figured out that the Meta regions were never closed and hbase
> > > would
> > > > stop only when logging in debug mode is enabled.
> > > >
> > > > Here is the code snippet from HResionServer.java on HBaseTrunk:
> > > >
> > > > else if (this.stopping && LOG.isDebugEnabled()) {
> > > >            LOG.info("Only meta regions remain open");
> > > >            if (!onlyMetaRegionsRemaining) {
> > > >              onlyMetaRegionsRemaining = isOnlyMetaRegionsRemaining();
> > > >            }
> > > >            if (onlyMetaRegionsRemaining) {
> > > >              // Set stopped if no requests since last time we went
> > around
> > > > the loop.
> > > >              // The remaining meta regions will be closed on our way
> > out.
> > > >              if (oldRequestCount == this.requestCount.get()) {
> > > >                stop("Stopped; only catalog regions remaining
> online");
> > > >                break;
> > > >              }
> > > >              oldRequestCount = this.requestCount.get();
> > > >            }
> > > >            LOG.debug("Waiting on " +
> > > getOnlineRegionsAsPrintableString());
> > > >   }
> > > >
> > > > I feel this condition is inappropriate. Can i remove this condition
> > check
> > > ?
> > > >
> > > > Regards,
> > > > Akash A
> > > >
> > >
> >
>

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