[jira] [Commented] (HBASE-5349) Automagically tweak global memstore and block cache sizes based on workload

["Ted Yu (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HBASE-5349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13760641#comment-13760641 ] 

Ted Yu commented on HBASE-5349:
-------------------------------

There're ^M's at the end of each line. Please generate patch on Linux.

For HeapMemoryAutoTuner:

It would be nice to put the patch on review board.
{code}
+      AutoTunerContext context = createTunerContext();^M
{code}
Do we need to create new context for each iteration of the chore ? Can one instance be reused ?
{code}
+        LOG.debug("From HeapMemoryBalancer new memstoreSize: " + memstoreSize + "%. new blockCacheSize: " + blockCacheSize + "%");^M
{code}
I think the percent sign is not needed.
{code}
+        if (1 - (memstoreSize + blockCacheSize) < HConstants.HBASE_CLUSTER_MINIMUM_MEMORY_THRESHOLD) {^M
+          LOG.warn("Current heap configuration from HeapMemoryBalancer exceeds "^M
+              + "the threshold required for successful cluster operation. "^M
{code}
Should some action be taken for the above case ? Otherwise tuning is effectively disabled.

Since memstoreSize and blockCacheSize are local variables, I would expect some action when result.needsTuning() returns true.

For DefaultHeapMemoryBalancerImpl, please add javadoc and audience annotation.
{code}
+      result.setMemstoreSize(context.getCurMemStoreSize() - step);^M
{code}
Should we check that the decrement would not produce negative result ?

Please add unit tests for the new classes.
                
> Automagically tweak global memstore and block cache sizes based on workload
> ---------------------------------------------------------------------------
>
>                 Key: HBASE-5349
>                 URL: https://issues.apache.org/jira/browse/HBASE-5349
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 0.92.0
>            Reporter: Jean-Daniel Cryans
>            Assignee: Anoop Sam John
>         Attachments: WIP_HBASE-5349.patch
>
>
> Hypertable does a neat thing where it changes the size given to the CellCache (our MemStores) and Block Cache based on the workload. If you need an image, scroll down at the bottom of this link: http://www.hypertable.com/documentation/architecture/
> That'd be one less thing to configure.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira