You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/24 21:35:26 UTC
[GitHub] [pulsar] michaeljmarshall opened a new pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
michaeljmarshall opened a new pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180
### Motivation
I recently researched the implementations of the `LoadSheddingStrategy` interface. I thought the Javadocs could be improved to make it easier for users/developers to better understand the classes.
### Modifications
* Add Javadoc for `ThresholdShedder`. I got some of my understanding for the code and some from https://github.com/apache/pulsar/pull/6772.
* Update Javadoc for `OverloadShedder`
### Verifying this change
I read through the code to write this documentation. It is possible that I misunderstood it, so I will request reviews from contributors that have written these classes.
### Does this pull request potentially affect one of the following parts:
No public/breaking changes.
### Documentation
This is an update to the docs.
--
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
[GitHub] [pulsar] Anonymitaet commented on a change in pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180#discussion_r716156123
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ThresholdShedder.java
##########
@@ -35,6 +35,18 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Load shedding strategy that will unload any broker that exceeds the average resource utilization of all brokers by a
Review comment:
```suggestion
* Load shedding strategy that unloads any broker that exceeds the average resource utilization of all brokers by a
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/OverloadShedder.java
##########
@@ -35,11 +35,14 @@
/**
* Load shedding strategy which will attempt to shed exactly one bundle on brokers which are overloaded, that is, whose
- * maximum system resource usage exceeds loadBalancerBrokerOverloadedThresholdPercentage. A bundle will be recommended
- * for unloading off that broker if and only if the following conditions hold: The broker has at least two bundles
- * assigned and the broker has at least one bundle that has not been unloaded recently according to
+ * maximum system resource usage exceeds loadBalancerBrokerOverloadedThresholdPercentage. To see which resources are
+ * considered when determining the maximum system resource, see {@link LocalBrokerData#getMaxResourceUsage()}. A bundle
+ * will be recommended for unloading off that broker if and only if the following conditions hold: The broker has at
+ * least two bundles assigned and the broker has at least one bundle that has not been unloaded recently according to
* LoadBalancerSheddingGracePeriodMinutes. The unloaded bundle will be the most expensive bundle in terms of message
- * rate that has not been recently unloaded.
+ * rate that has not been recently unloaded. Note that this strategy does not take into account "underloaded" brokers
+ * when determining which bundles to unload. If you are looking for a strategy that will spread load evenly across
Review comment:
```suggestion
* when determining which bundles to unload. If you are looking for a strategy that spreads load evenly across
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ThresholdShedder.java
##########
@@ -35,6 +35,18 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Load shedding strategy that will unload any broker that exceeds the average resource utilization of all brokers by a
+ * configured threshold. As a consequence, this strategy tends to distribute load among all brokers. It does this by
+ * first computing the average resource usage per broker for the whole cluster. The resource usage for each broker is
+ * calculated using the following method: {@link LocalBrokerData#getMaxResourceUsageWithWeight)}. The weights for each
+ * resource are configurable. Historical observations are included in the running average based on the broker's
+ * setting for loadBalancerHistoryResourcePercentage. Once the average resource usage is calculated, a broker's
+ * current/historical usage is compared to the average broker usage. If a broker's usage is greater than the average
+ * usage per broker plus the loadBalancerBrokerThresholdShedderPercentage, this load shedder will propose removing
Review comment:
```suggestion
* usage per broker plus the loadBalancerBrokerThresholdShedderPercentage, this load shedder proposes removing
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ThresholdShedder.java
##########
@@ -35,6 +35,18 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Load shedding strategy that will unload any broker that exceeds the average resource utilization of all brokers by a
+ * configured threshold. As a consequence, this strategy tends to distribute load among all brokers. It does this by
+ * first computing the average resource usage per broker for the whole cluster. The resource usage for each broker is
+ * calculated using the following method: {@link LocalBrokerData#getMaxResourceUsageWithWeight)}. The weights for each
+ * resource are configurable. Historical observations are included in the running average based on the broker's
+ * setting for loadBalancerHistoryResourcePercentage. Once the average resource usage is calculated, a broker's
+ * current/historical usage is compared to the average broker usage. If a broker's usage is greater than the average
+ * usage per broker plus the loadBalancerBrokerThresholdShedderPercentage, this load shedder will propose removing
+ * enough bundles to bring the unloaded broker 5% below the current average broker usage. Note that recently
+ * unloaded bundles will not be unloaded again.
Review comment:
```suggestion
* unloaded bundles are not unloaded again.
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/OverloadShedder.java
##########
@@ -35,11 +35,14 @@
/**
* Load shedding strategy which will attempt to shed exactly one bundle on brokers which are overloaded, that is, whose
- * maximum system resource usage exceeds loadBalancerBrokerOverloadedThresholdPercentage. A bundle will be recommended
- * for unloading off that broker if and only if the following conditions hold: The broker has at least two bundles
- * assigned and the broker has at least one bundle that has not been unloaded recently according to
+ * maximum system resource usage exceeds loadBalancerBrokerOverloadedThresholdPercentage. To see which resources are
+ * considered when determining the maximum system resource, see {@link LocalBrokerData#getMaxResourceUsage()}. A bundle
+ * will be recommended for unloading off that broker if and only if the following conditions hold: The broker has at
Review comment:
```suggestion
* is recommended for unloading off that broker if and only if the following conditions hold: The broker has at
```
--
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
[GitHub] [pulsar] michaeljmarshall commented on pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180#issuecomment-926932504
@hangc0276 - PTAL, since you were the original author of the `ThresholdShedder`.
--
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
[GitHub] [pulsar] Anonymitaet merged pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180
--
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
[GitHub] [pulsar] Anonymitaet merged pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180
--
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
[GitHub] [pulsar] michaeljmarshall commented on pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180#issuecomment-929655768
@Anonymitaet - PTAL. The tests are passing now. Thanks!
--
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
[GitHub] [pulsar] michaeljmarshall commented on pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180#issuecomment-929643427
--
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
[GitHub] [pulsar] michaeljmarshall commented on pull request #12180: [Docs][LoadBalancing] Add/Improve Javadocs for LoadSheddingStrategy impls
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #12180:
URL: https://github.com/apache/pulsar/pull/12180#issuecomment-929643427
/pulsarbot run-failure-checks
--
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