[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17598: [improve][loadbalance] added loadBalancerReportUpdateMemoryUsageChecked and ignores memory usage in getMaxResourceUsage()

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

codelipenghui commented on code in PR #17598:
URL: https://github.com/apache/pulsar/pull/17598#discussion_r969625275


##########
pulsar-common/src/main/java/org/apache/pulsar/policies/data/loadbalancer/LocalBrokerData.java:
##########
@@ -235,7 +235,8 @@ private void updateBundleData(final Map<String, NamespaceBundleStats> bundleStat
     }
 
     public double getMaxResourceUsage() {
-        return max(cpu.percentUsage(), memory.percentUsage(), directMemory.percentUsage(), bandwidthIn.percentUsage(),
+        // does not consider memory because it is noisy by gc.
+        return max(cpu.percentUsage(), directMemory.percentUsage(), bandwidthIn.percentUsage(),

Review Comment:
   Could you please help add a unit test?
   Looks like we changed the behavior, but all the tests got passed.
   



##########
conf/broker.conf:
##########
@@ -1163,6 +1163,9 @@ loadBalancerEnabled=true
 # Percentage of change to trigger load report update
 loadBalancerReportUpdateThresholdPercentage=10
 
+# minimum interval to update load report
+loadBalancerReportUpdateMinIntervalMilliSeconds=5000

Review Comment:
   ```suggestion
   loadBalancerReportUpdateMinIntervalMillis=5000
   ```
   
   Just keep consistent with other configuration names in the broker.conf



##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2087,6 +2087,14 @@ public class ServiceConfiguration implements PulsarConfiguration {
         category = CATEGORY_LOAD_BALANCER,
         doc = "maximum interval to update load report"
     )
+
+    private int loadBalancerReportUpdateMinIntervalMilliSeconds = 5000;
+    @FieldContext(
+            category = CATEGORY_LOAD_BALANCER,
+            dynamic = true,
+            doc = "Min delay of load report to collect, in milli-seconds"
+    )

Review Comment:
   Should move to line 2090? 



-- 
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