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 > > > > > > > > > >
