Apache9 commented on code in PR #6897:
URL: https://github.com/apache/hbase/pull/6897#discussion_r2040553045
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -911,6 +900,22 @@ boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey
blockCacheKey, BucketEnt
return false;
}
+ @Override
+ public void onConfigurationChange(Configuration config) {
+ this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME,
DEFAULT_ACCEPT_FACTOR);
+ this.minFactor = conf.getFloat(MIN_FACTOR_CONFIG_NAME, DEFAULT_MIN_FACTOR);
+ this.extraFreeFactor = conf.getFloat(EXTRA_FREE_FACTOR_CONFIG_NAME,
DEFAULT_EXTRA_FREE_FACTOR);
+ this.singleFactor = conf.getFloat(SINGLE_FACTOR_CONFIG_NAME,
DEFAULT_SINGLE_FACTOR);
+ this.multiFactor = conf.getFloat(MULTI_FACTOR_CONFIG_NAME,
DEFAULT_MULTI_FACTOR);
+ this.memoryFactor = conf.getFloat(MEMORY_FACTOR_CONFIG_NAME,
DEFAULT_MEMORY_FACTOR);
+ this.queueAdditionWaitTime =
+ conf.getLong(QUEUE_ADDITION_WAIT_TIME, DEFAULT_QUEUE_ADDITION_WAIT_TIME);
+ this.bucketcachePersistInterval =
conf.getLong(BUCKETCACHE_PERSIST_INTERVAL_KEY, 1000);
+ this.persistenceChunkSize =
+ conf.getLong(BACKING_MAP_PERSISTENCE_CHUNK_SIZE,
DEFAULT_BACKING_MAP_PERSISTENCE_CHUNK_SIZE);
+ sanityCheckConfigs();
Review Comment:
Why not put the above check here?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -911,6 +900,22 @@ boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey
blockCacheKey, BucketEnt
return false;
}
+ @Override
+ public void onConfigurationChange(Configuration config) {
+ this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME,
DEFAULT_ACCEPT_FACTOR);
Review Comment:
Haven't touch this area for a long time, all these configs can be loaded
dynamically? IIRC, BucketCache will pre alloc all the memory chunks when
initializing, configuration refreshing can not trigger the allocation again.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -450,6 +436,9 @@ private void sanityCheckConfigs() {
Preconditions.checkArgument((singleFactor + multiFactor + memoryFactor) ==
1,
SINGLE_FACTOR_CONFIG_NAME + ", " + MULTI_FACTOR_CONFIG_NAME + ", and "
+ MEMORY_FACTOR_CONFIG_NAME + " segments must add up to 1.0");
+ if (this.persistenceChunkSize <= 0) {
Review Comment:
Is this change related?
--
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]
