taklwu commented on a change in pull request #3684:
URL: https://github.com/apache/hbase/pull/3684#discussion_r711482626
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java
##########
@@ -216,6 +216,9 @@ public static float getBlockCacheHeapPercent(final
Configuration conf) {
public static long getOnHeapCacheSize(final Configuration conf) {
float cachePercentage =
conf.getFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY,
HConstants.HFILE_BLOCK_CACHE_SIZE_DEFAULT);
+ // only uses when it's ClientSideRegionScanner or user explicitly sets it
+ long fixedCacheSize =
conf.getLong(HConstants.HBASE_BLOCK_CACHE_FIXED_SIZE_KEY,
Review comment:
mmm, I think you're referring to the HeapMemoryTunerChore#tune for any
RS daemon, and assuming the user of this flag are using for RS, they should be
fine and this tuner will tune it (global memstore) back to under 80%. (here the
block cache max size could be 100% of the max heap)
for the client scanner use cases, let me set the size ratio won't go beyond
`hfile.block.cache.size` key such it still respect the range of 0.0001f and
1.0f of current max heap.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/ResizableBlockCache.java
##########
@@ -31,4 +31,13 @@
* @param size The max heap size.
*/
void setMaxSize(long size);
+
+ /**
+ * Check if block type is meta or index block
+ * @param blockType block type of a given HFile block
+ * @return true if block type is non-data block
+ */
+ default boolean isMetaBlock(BlockType blockType) {
Review comment:
moved
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/ResizableBlockCache.java
##########
@@ -31,4 +31,13 @@
* @param size The max heap size.
*/
void setMaxSize(long size);
+
+ /**
+ * Check if block type is meta or index block
+ * @param blockType block type of a given HFile block
+ * @return true if block type is non-data block
+ */
+ default boolean isMetaBlock(BlockType blockType) {
+ return blockType.getCategory() != BlockType.BlockCategory.DATA;
Review comment:
make senses, I will use `null` here.
I wanna put this note here, where `blockType` is null, any followup use
cases from the parent caller with `getBlockType()` and
`CombinedBlockCache#getBlock` becomes strange and the system must be wrong; It
does not tell the difference between a bull block or a real data block type. (I
somehow want to throw null pointer exception instead to tell the system is
wrong, but let's keep it safe and don't break the vehicle because of this tiny
hole from any upstream)
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
##########
@@ -1049,6 +1049,16 @@
public static final float HFILE_BLOCK_CACHE_SIZE_DEFAULT = 0.4f;
+ /**
+ * Configuration key for setting the fix size of the block size, default do
nothing and it should
+ * be explicitly set by user or only used within ClientSideRegionScanner. if
it's set less than
+ * current max on heap size, it overrides the max size of block cache
+ */
+ public static final String HBASE_BLOCK_CACHE_FIXED_SIZE_KEY =
+ "hfile.block.cache.fixed.size";
Review comment:
how about `hfile.onheap.block.cache.fixed.size` ? sorry that I'm bad at
naming ...
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
##########
@@ -1049,6 +1049,16 @@
public static final float HFILE_BLOCK_CACHE_SIZE_DEFAULT = 0.4f;
+ /**
+ * Configuration key for setting the fix size of the block size, default do
nothing and it should
+ * be explicitly set by user or only used within ClientSideRegionScanner. if
it's set less than
+ * current max on heap size, it overrides the max size of block cache
+ */
+ public static final String HBASE_BLOCK_CACHE_FIXED_SIZE_KEY =
Review comment:
yeah, I'm also struggling about this new constant, IMO making it
configurable such if anything happens, operator still have a chance to save the
world....tho it's very unlikely we want others to modify this value.
so, again, as Josh suggested, we want this to be tiny with 32MB or less, and
maybe as you said, if the current heap does not have enough size even with
setting this tiny 32MB, we use the 20% of whatever available on heap and ignore
this configuration.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/client/ClientSideRegionScanner.java
##########
@@ -60,6 +62,15 @@ public ClientSideRegionScanner(Configuration conf,
FileSystem fs,
region = HRegion.newHRegion(CommonFSUtils.getTableDir(rootDir,
htd.getTableName()), null, fs,
conf, hri, htd, null);
region.setRestoredRegion(true);
+ // non RS process does not have a block cache, and this a client side
scanner,
+ // create one for MapReduce jobs to cache the INDEX block by setting to use
+ // IndexOnlyLruBlockCache and set a value to
HBASE_CLIENT_SCANNER_BLOCK_CACHE_SIZE_KEY
+ conf.set(BlockCacheFactory.BLOCKCACHE_POLICY_KEY, "IndexOnlyLru");
Review comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
