[ https://issues.apache.org/jira/browse/HBASE-5349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13760641#comment-13760641 ]
Ted Yu edited comment on HBASE-5349 at 9/6/13 9:20 PM: ------------------------------------------------------- There're ^M's at the end of each line. Please generate patch on Linux. It would be nice to put the patch on review board. For HeapMemoryAutoTuner: {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. was (Author: yuzhih...@gmail.com): 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