[GitHub] [pulsar] Jason918 commented on a change in pull request #13679: Support dynamic update cache config

[GitBox <gi...@apache.org>]

Jason918 commented on a change in pull request #13679:
URL: https://github.com/apache/pulsar/pull/13679#discussion_r825419419



##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1678,23 +1678,34 @@
     private int managedLedgerMaxAckQuorum = 5;
     @FieldContext(
         category = CATEGORY_STORAGE_ML,
+        dynamic = true,
         doc = "Amount of memory to use for caching data payload in managed ledger. \n\nThis"
             + " memory is allocated from JVM direct memory and it's shared across all the topics"
             + " running in the same broker. By default, uses 1/5th of available direct memory")
     private int managedLedgerCacheSizeMB = Math.max(64,
             (int) (io.netty.util.internal.PlatformDependent.maxDirectMemory() / 5 / (1024 * 1024)));
+
+    @FieldContext(
+            category = CATEGORY_STORAGE_ML,
+            dynamic = true,
+            doc = "Threshold to evicteis triggered"
+    )
+    private double evictionTriggerThresholdPercent = 0.98;

Review comment:
       Normally, we may need a PIP to add new config. And you should provide solid reason to make it configurable. 

##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java
##########
@@ -59,7 +63,8 @@
 
     protected static final double MB = 1024 * 1024;
 
-    private static final double evictionTriggerThresholdPercent = 0.98;
+    @Setter
+    private volatile double evictionTriggerThresholdPercent = 0.98;

Review comment:
       This is already defined in config with init value. Can we remove it from this class ？ 




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org