[ 
https://issues.apache.org/jira/browse/HADOOP-2636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12563761#action_12563761
 ] 

Jim Kellerman commented on HADOOP-2636:
---------------------------------------

> stack commented on HADOOP-2636:
> -------------------------------
> 
> In HLogKey, was it just a case of a misnamed data member?  
> All along it was a store but we were calling it region?  See below:
> 
> {code}
> -  Text regionName = new Text();
> +  Text storeName = new Text();
> {code}

No. Before flushes happened at the region level and we wrote the region name 
into the log.
Moving flushes to the store level required writing the store name instead of 
the region name
into the log so that log rolling and garbage collection would work properly.

Note, however that this is an incompatible change, so before upgrading, all 
region servers
should be shut down cleanly so that there are no logs to recover.
 
> Can this string creation be avoided in HStore; e.g. can 
> storeName be Text?
> 
> {code}
> +            || 
> (key.getStoreName().toString().compareTo(storeName) != 
> + 0)
> {code}

Done.
 
> Logging below at INFO level seems inappropriate:
> 
> {code}
> +      LOG.info("Not flushing cache for " + storeName +
> +          " because it has 0 entries");
> {code}

Changed to debug level.

> This kind of logging doesn't help (though I think this log is 
> just a line moved from elsewhere):
> 
> {code}
> +            LOG.debug("nothing to compact for " + this.storeName);
> {code}
>
> Should say why there is nothing to compact -- e.g. only one 
> file present or holds references.

Fixed.
 
> Just remove rather than comment out?
> 
> {code}
> -      HStoreKey rowKey = new HStoreKey(row, timestamp);
> +/*      HStoreKey rowKey = new HStoreKey(row, timestamp); */
> {code}

No. This line is a part of other code that needs to be enabled when 
MapFile.Reader.getClosest before is implemented.
 
> HStoreSize inner class is no longer needed because the check 
> is local to HStore where before it was higher up in HRegion?  
> The info HStoreSize carried is now all availble in the 
> context where the check is being done?

Yes.
 
> Nice how you cleaned up lease-making/updating.

Thanks.
 
> Why make RowMap non-private?  Its used by inner classes?

Synthetic accessors showed up in eclipse.
 
> The below no longer makes use of TextSequences?  Any reason 
> for that?  (TS was means of cutting down on object creations. 
> Profiling, using TSs made a big difference).
> {code}
> -      Text qualifier = HStoreKey.extractQualifier(col);
> +      Text member = HStoreKey.extractMember(col);
> {code}

I just ran into too many instances where things broke because TextSequence is 
not a Text.
Text should either use accessors to access members of another Text or it should 
just be
declared as final because it cannot be subclassed as it stands now.
 

> [hbase] Make cache flush triggering less simplistic
> ---------------------------------------------------
>
>                 Key: HADOOP-2636
>                 URL: https://issues.apache.org/jira/browse/HADOOP-2636
>             Project: Hadoop Core
>          Issue Type: Improvement
>          Components: contrib/hbase
>    Affects Versions: 0.16.0
>            Reporter: stack
>            Assignee: Jim Kellerman
>             Fix For: 0.17.0
>
>         Attachments: patch.txt, patch.txt, patch.txt
>
>
> When flusher runs -- its triggered when the sum of all Stores in a Region > a 
> configurable max size -- we flush all Stores though a Store memcache might 
> have but a few bytes.
> I would think Stores should only dump their memcache disk if they have some 
> substance.
> The problem becomes more acute, the more families you have in a Region.
> Possible behaviors would be to dump the biggest Store only, or only those 
> Stores > 50% of max memcache size.  Behavior would vary dependent on the 
> prompt that provoked the flush.  Would also log why the flush is running: 
> optional or > max size.
> This issue comes out of HADOOP-2621.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to