I agree. On Fri, Jun 24, 2011 at 7:36 AM, Akash Ashok <[email protected]> wrote:
> 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 <[email protected]> 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 <[email protected]> > > 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 <[email protected]> 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 <[email protected] > > > > > > 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 > > > > > > > > > > > > > > >
