-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[improve][broker] PIP-192 Moved the common broker load data feature(weightedMaxEMA) to BrokerLoadData #19154
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,10 +19,8 @@ | |
package org.apache.pulsar.broker.loadbalance.extensions.strategy; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.Set; | ||
import java.util.concurrent.ThreadLocalRandom; | ||
|
@@ -39,81 +37,37 @@ | |
*/ | ||
@Slf4j | ||
public class LeastResourceUsageWithWeight implements BrokerSelectionStrategy { | ||
private static final double MAX_RESOURCE_USAGE = 1.0d; | ||
// Maintain this list to reduce object creation. | ||
private final ArrayList<String> bestBrokers; | ||
private final Set<String> noLoadDataBrokers; | ||
private final Map<String, Double> brokerAvgResourceUsageWithWeight; | ||
|
||
public LeastResourceUsageWithWeight() { | ||
this.bestBrokers = new ArrayList<>(); | ||
this.brokerAvgResourceUsageWithWeight = new HashMap<>(); | ||
this.noLoadDataBrokers = new HashSet<>(); | ||
} | ||
|
||
// A broker's max resource usage with weight using its historical load and short-term load data with weight. | ||
private double getMaxResourceUsageWithWeight(final String broker, final BrokerLoadData brokerLoadData, | ||
final ServiceConfiguration conf, boolean debugMode) { | ||
final double overloadThreshold = conf.getLoadBalancerBrokerOverloadedThresholdPercentage() / 100.0; | ||
final double maxUsageWithWeight = | ||
updateAndGetMaxResourceUsageWithWeight(broker, brokerLoadData, conf, debugMode); | ||
final var maxUsageWithWeight = brokerLoadData.getWeightedMaxEMA(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for this comment. I changed this Ideally, the downstream logics using this Let's ensure we publish Also, accordingly, I cleaned up |
||
|
||
|
||
if (maxUsageWithWeight > overloadThreshold) { | ||
log.warn( | ||
"Broker {} is overloaded, max resource usage with weight percentage: {}%, " | ||
+ "CPU: {}%, MEMORY: {}%, DIRECT MEMORY: {}%, BANDWIDTH IN: {}%, " | ||
+ "BANDWIDTH OUT: {}%, CPU weight: {}, MEMORY weight: {}, DIRECT MEMORY weight: {}, " | ||
+ "BANDWIDTH IN weight: {}, BANDWIDTH OUT weight: {}", | ||
broker, maxUsageWithWeight * 100, | ||
brokerLoadData.getCpu().percentUsage(), brokerLoadData.getMemory().percentUsage(), | ||
brokerLoadData.getDirectMemory().percentUsage(), brokerLoadData.getBandwidthIn().percentUsage(), | ||
brokerLoadData.getBandwidthOut().percentUsage(), conf.getLoadBalancerCPUResourceWeight(), | ||
conf.getLoadBalancerMemoryResourceWeight(), conf.getLoadBalancerDirectMemoryResourceWeight(), | ||
conf.getLoadBalancerBandwithInResourceWeight(), | ||
conf.getLoadBalancerBandwithOutResourceWeight()); | ||
"Broker {} is overloaded, brokerLoad({}%) > overloadThreshold({}%). load data:{{}}", | ||
broker, | ||
maxUsageWithWeight * 100, | ||
overloadThreshold * 100, | ||
brokerLoadData.toString(conf)); | ||
} else if (debugMode) { | ||
log.info("Broker {} load data:{{}}", broker, brokerLoadData.toString(conf)); | ||
} | ||
|
||
if (debugMode) { | ||
log.info("Broker {} has max resource usage with weight percentage: {}%", | ||
broker, maxUsageWithWeight * 100); | ||
} | ||
|
||
return maxUsageWithWeight; | ||
} | ||
|
||
/** | ||
* Update and get the max resource usage with weight of broker according to the service configuration. | ||
* | ||
* @param broker The broker name. | ||
* @param brokerData The broker load data. | ||
* @param conf The service configuration. | ||
* @param debugMode The debug mode to print computed load states and decision information. | ||
* @return the max resource usage with weight of broker | ||
*/ | ||
private double updateAndGetMaxResourceUsageWithWeight(String broker, BrokerLoadData brokerData, | ||
ServiceConfiguration conf, boolean debugMode) { | ||
final double historyPercentage = conf.getLoadBalancerHistoryResourcePercentage(); | ||
Double historyUsage = brokerAvgResourceUsageWithWeight.get(broker); | ||
double resourceUsage = brokerData.getMaxResourceUsageWithWeight( | ||
conf.getLoadBalancerCPUResourceWeight(), | ||
conf.getLoadBalancerMemoryResourceWeight(), | ||
conf.getLoadBalancerDirectMemoryResourceWeight(), | ||
conf.getLoadBalancerBandwithInResourceWeight(), | ||
conf.getLoadBalancerBandwithOutResourceWeight()); | ||
historyUsage = historyUsage == null | ||
? resourceUsage : historyUsage * historyPercentage + (1 - historyPercentage) * resourceUsage; | ||
if (debugMode) { | ||
log.info( | ||
"Broker {} get max resource usage with weight: {}, history resource percentage: {}%, CPU weight: " | ||
+ "{}, MEMORY weight: {}, DIRECT MEMORY weight: {}, BANDWIDTH IN weight: {}, BANDWIDTH " | ||
+ "OUT weight: {} ", | ||
broker, historyUsage, historyPercentage, conf.getLoadBalancerCPUResourceWeight(), | ||
conf.getLoadBalancerMemoryResourceWeight(), conf.getLoadBalancerDirectMemoryResourceWeight(), | ||
conf.getLoadBalancerBandwithInResourceWeight(), | ||
conf.getLoadBalancerBandwithOutResourceWeight()); | ||
} | ||
Comment on lines
-104
to
-113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the debug log be removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. I think we can keep this. Added BrokerLoadData.toString() and added the debug log.
|
||
brokerAvgResourceUsageWithWeight.put(broker, historyUsage); | ||
return historyUsage; | ||
} | ||
|
||
/** | ||
* Find a suitable broker to assign the given bundle to. | ||
|
@@ -143,7 +97,7 @@ public Optional<String> select(List<String> candidates, ServiceUnitId bundleToAs | |
for (String broker : candidates) { | ||
var brokerLoadDataOptional = context.brokerLoadDataStore().get(broker); | ||
if (brokerLoadDataOptional.isEmpty()) { | ||
log.warn("There is no broker load data for broker:{}. Skipping this broker.", broker); | ||
log.warn("There is no broker load data for broker:{}. Skipping this broker. Phase one", broker); | ||
noLoadDataBrokers.add(broker); | ||
continue; | ||
} | ||
|
@@ -162,12 +116,17 @@ public Optional<String> select(List<String> candidates, ServiceUnitId bundleToAs | |
if (debugMode) { | ||
log.info("Computed avgUsage:{}, diffThreshold:{}", avgUsage, diffThreshold); | ||
} | ||
candidates.forEach(broker -> { | ||
Double avgResUsage = brokerAvgResourceUsageWithWeight.getOrDefault(broker, MAX_RESOURCE_USAGE); | ||
for (String broker : candidates) { | ||
var brokerLoadDataOptional = context.brokerLoadDataStore().get(broker); | ||
if (brokerLoadDataOptional.isEmpty()) { | ||
log.warn("There is no broker load data for broker:{}. Skipping this broker. Phase two", broker); | ||
continue; | ||
} | ||
double avgResUsage = brokerLoadDataOptional.get().getWeightedMaxEMA(); | ||
if ((avgResUsage + diffThreshold <= avgUsage && !noLoadDataBrokers.contains(broker))) { | ||
bestBrokers.add(broker); | ||
} | ||
}); | ||
} | ||
} | ||
|
||
if (bestBrokers.isEmpty()) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does EMA mean? Is it better to add a description for "EMA"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an exponential moving average. Updated the comment for the weightedMaxEMA variable.