michaeljmarshall commented on a change in pull request #13609:
URL: https://github.com/apache/pulsar/pull/13609#discussion_r778423016
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LinuxBrokerHostUsageImpl.java
##########
@@ -84,11 +87,12 @@ public LinuxBrokerHostUsageImpl(int
hostUsageCheckIntervalMin,
log.warn("Failed to check cgroup CPU usage file: {}",
e.getMessage());
}
this.isCGroupsEnabled = isCGroupsEnabled;
+ this.executorService = executorService;
+ this.hostUsageCheckIntervalMin = hostUsageCheckIntervalMin;
// Call now to initialize values before the constructor returns
calculateBrokerHostUsage();
-
executorService.scheduleAtFixedRate(catchingAndLoggingThrowables(this::calculateBrokerHostUsage),
- hostUsageCheckIntervalMin,
+
this.executorService.schedule(catchingAndLoggingThrowables(this::calculateBrokerHostUsage),
Review comment:
I also wonder why we use `scheduleAtFixedRate` instead of
`scheduleAtFixedDelay`. When we have long GC, it's not helpful to run
`calculateBrokerHostUsage()` many times subsequently in order to keep a
constant rate. @lhotari and @merlimat - what do you think?
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LinuxBrokerHostUsageImpl.java
##########
@@ -128,6 +132,9 @@ public void calculateBrokerHostUsage() {
lastCollection = System.currentTimeMillis();
this.usage = usage;
usage.setCpu(new ResourceUsage(cpuUsage, totalCpuLimit));
+
+
executorService.schedule(catchingAndLoggingThrowables(this::calculateBrokerHostUsage),
Review comment:
Note also that this PR's implementation is essentially proposing that we
move to run with a fixed delay.
--
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]
