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 2022/12/10 14:44:15 UTC
[GitHub] [pulsar] heesung-sn opened a new pull request, #18865: [improve][broker] PIP-220 Added TransferShedder
heesung-sn opened a new pull request, #18865:
URL: https://github.com/apache/pulsar/pull/18865
<!--
### Contribution Checklist
- PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
-->
<!-- or this PR is one task of an issue -->
Master Issue: https://github.com/apache/pulsar/issues/18215
### Motivation
<!-- Explain here the context, and why you're making that change. What is the problem you're trying to solve. -->
This PR implement PIP-220, https://github.com/apache/pulsar/issues/18215
### Modifications
For the PIP-220(a sub-pip of pip-192, https://github.com/apache/pulsar/issues/16691), this PR implements the TransferShedder and its unit test.
Also, there are minor changes in the related classes:
- Added the `lastUpdatedAt` member var in BrokerLoadData.
- Added the `recentlyUnloadedBrokers` input var in NamespaceUnloadStrategy.findBundlesForUnloading(...)
- Added the TransferShedder-related configs in ServiceConfiguration
<!-- Describe the modifications you've done. -->
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- *Added unit tests for the new classes.
### Does this pull request potentially affect one of the following parts:
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
We will have separate PRs to update the Doc later.
### Matching PR in forked repository
PR in forked repository: https://github.com/heesung-sn/pulsar/pull/16
<!--
After opening this PR, the build in apache/pulsar will fail and instructions will
be provided for opening a PR in the PR author's forked repository.
apache/pulsar pull requests should be first tested in your own fork since the
apache/pulsar CI based on GitHub Actions has constrained resources and quota.
GitHub Actions provides separate quota for pull requests that are executed in
a forked repository.
The tests will be run in the forked repository until all PR review comments have
been handled, the tests pass and the PR is approved by a reviewer.
-->
--
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] heesung-sn commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1094001823
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
That's another way of fine-configuring the log level, and I assume we can control the log level by the package as well.
But with this dynamic config, we can more easily turn on/off the debugging logs too.
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093982471
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
Does this config is necessary? Maybe we can do the same thing via modify the broker log config file.
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
Does this config is necessary? Maybe we can do the same thing via modifying the broker log config file.
--
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] heesung-sn commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093985502
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
Some systems do support fine-grained debugging logs for certain features.
I am curious if the pulsar follows this practice, but I think this could be useful to see debug logs from load balancer logic only.
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1094053463
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
OK, but it's a little strange to add a separate param to control debug log information.
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093982471
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
Does this config is necessary? Maybe we can do the same thing via modify log configurations.
--
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] heesung-sn commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093991281
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
I think we can mark all these configs as dynamic. Updating.
--
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] heesung-sn commented on pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#issuecomment-1412858746
Please continue the review. I applied the changes from https://github.com/apache/pulsar/pull/19154.
--
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] Demogorgon314 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1046922944
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2431,6 +2431,53 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for TransferShedder. The debug mode prints more logs. "
+ + "(only used by TransferSheddeer)"
+ )
+ private boolean loadBalancerExtentionsTransferShedderDebugModeEnabled = false;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "The target standard deviation of the resource usage across brokers "
+ + "(100% resource usage is 1.0 load)."
+ + "The shedder logic tries to distribute bundle load across brokers to meet this target std. "
+ + "(only used by TransferSheddeer)"
+ )
+ private double loadBalancerExtentionsTransferShedderTargetLoadStd = 0.25;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the bundle transfer mode. "
+ + "The transfer mode transfers bundles from source brokers to destination brokers. "
+ + "(only used by TransferSheddeer)"
+ )
+ private boolean loadBalancerExtentionsTransferShedderTransferEnabled = true;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "maximum number of brokers to transfer bundle load for each unloading cycle."
+ + " (only used by TransferSheddeer)"
+ )
+ private int loadBalancerExtentionsTransferShedderMaxNumberOfBrokerTransfersPerCycle = 3;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "minimum waiting time (in seconds) to update the next broker load data after transfers. "
+ + "The logic tries to give enough time for each broker to recompute its load after unloading. "
+ + "(only used by TransferSheddeer)"
+ )
+ private long loadBalancerExtentionsTransferShedderBrokerLoadDataUpdateMinWaitingTimeAfterUnloadingInSeconds = 180;
Review Comment:
This configuration name is so long... Do we have a better choice?
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093997621
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
The Pulsar broker uses the log4j2, which supports custom log configurations by modifying the config file `${PULSAR_HOME}/conf/log4j2.yaml`, which supports log control at the class level.
--
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] Demogorgon314 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1046922944
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2431,6 +2431,53 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for TransferShedder. The debug mode prints more logs. "
+ + "(only used by TransferSheddeer)"
+ )
+ private boolean loadBalancerExtentionsTransferShedderDebugModeEnabled = false;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "The target standard deviation of the resource usage across brokers "
+ + "(100% resource usage is 1.0 load)."
+ + "The shedder logic tries to distribute bundle load across brokers to meet this target std. "
+ + "(only used by TransferSheddeer)"
+ )
+ private double loadBalancerExtentionsTransferShedderTargetLoadStd = 0.25;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the bundle transfer mode. "
+ + "The transfer mode transfers bundles from source brokers to destination brokers. "
+ + "(only used by TransferSheddeer)"
+ )
+ private boolean loadBalancerExtentionsTransferShedderTransferEnabled = true;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "maximum number of brokers to transfer bundle load for each unloading cycle."
+ + " (only used by TransferSheddeer)"
+ )
+ private int loadBalancerExtentionsTransferShedderMaxNumberOfBrokerTransfersPerCycle = 3;
+
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "minimum waiting time (in seconds) to update the next broker load data after transfers. "
+ + "The logic tries to give enough time for each broker to recompute its load after unloading. "
+ + "(only used by TransferSheddeer)"
+ )
+ private long loadBalancerExtentionsTransferShedderBrokerLoadDataUpdateMinWaitingTimeAfterUnloadingInSeconds = 180;
Review Comment:
This configuration name is so long...
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1094053463
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
OK, but it's a little strange to add a separate param to control debug log information, we also need to add extra judgment logic.
--
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] heesung-sn commented on pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by GitBox <gi...@apache.org>.
heesung-sn commented on PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#issuecomment-1376649924
Please hold on reviewing this PR. I will refactor this PR according to the changes in https://github.com/apache/pulsar/pull/19154.
--
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] gaoran10 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093997621
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
The Pulsar broker uses the log4j2, which supports custom log configurations by modifying the log config file `${PULSAR_HOME}/conf/log4j2.yaml`, which supports log control at class level.
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
The Pulsar broker uses the log4j2, which supports custom log configurations by modifying the log config file `${PULSAR_HOME}/conf/log4j2.yaml`, which supports log control at class level.
--
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] merlimat merged pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #18865:
URL: https://github.com/apache/pulsar/pull/18865
--
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] Demogorgon314 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by "Demogorgon314 (via GitHub)" <gi...@apache.org>.
Demogorgon314 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1093988251
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2450,6 +2450,64 @@ The delayed message index bucket time step(in seconds) in per bucket snapshot se
)
private long namespaceBundleUnloadingTimeoutMs = 60000;
+ @FieldContext(
+ category = CATEGORY_LOAD_BALANCER,
+ doc = "Option to enable the debug mode for the load balancer logics. "
+ + "The debug mode prints more logs to provide more information "
+ + "such as load balance states and decisions. "
+ + "(only used in load balancer extension logics)"
+ )
+ private boolean loadBalancerDebugModeEnabled = false;
Review Comment:
Should we mark this configuration as dynamic? Then we can change this configuration in real time.
--
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] Demogorgon314 commented on a diff in pull request #18865: [improve][broker] PIP-220 Added TransferShedder
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on code in PR #18865:
URL: https://github.com/apache/pulsar/pull/18865#discussion_r1048555186
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/scheduler/TransferShedder.java:
##########
@@ -0,0 +1,414 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.loadbalance.extensions.scheduler;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.MinMaxPriorityQueue;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import lombok.ToString;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.mutable.MutableBoolean;
+import org.apache.commons.lang3.mutable.MutableDouble;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.pulsar.broker.PulsarService;
+import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.loadbalance.extensions.LoadManagerContext;
+import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLoadData;
+import org.apache.pulsar.broker.loadbalance.extensions.data.TopBundlesLoadData;
+import org.apache.pulsar.broker.loadbalance.extensions.models.Unload;
+import org.apache.pulsar.broker.loadbalance.extensions.store.LoadDataStore;
+import org.apache.pulsar.broker.loadbalance.impl.LoadManagerShared;
+import org.apache.pulsar.broker.loadbalance.impl.SimpleResourceAllocationPolicies;
+import org.apache.pulsar.common.naming.NamespaceName;
+import org.apache.pulsar.metadata.api.MetadataStoreException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Load shedding strategy that unloads bundles from the highest loaded brokers.
+ * This strategy is only configurable in the broker load balancer extenstions introduced by
+ * PIP-192[https://github.com/apache/pulsar/issues/16691].
+ *
+ * This load shedding strategy has the following goals:
+ * 1. Distribute bundle load across brokers in order to make the standard deviation of the avg resource usage,
+ * std(exponential-moving-avg(max(cpu, memory, network, throughput)) for each broker) below the target,
+ * configurable by loadBalancerExtentionsTransferShedderTargetLoadStd.
+ * 2. Use the transfer protocol to transfer bundle load from the highest loaded to the lowest loaded brokers,
+ * if configured by loadBalancerExtentionsTransferShedderTransferEnabled=true.
+ * 3. Avoid repeated bundle unloading by recomputing historical broker resource usage after unloading and also
+ * skipping the bundles that are recently unloaded.
+ * 4. Prioritize unloading bundles to underloaded brokers when their message throughput is zero(new brokers).
+ * 5. Do not use outdated broker load data
+ * (configurable by loadBalancerExtentionsTransferShedderLoadUpdateMaxWaitingTimeInSeconds).
+ * 6. Give enough time for each broker to recompute its load after unloading
+ * (configurable by loadBalancerExtentionsTransferShedderBrokerLoadDataUpdateMinWaitingTimeAfterUnloadingInSeconds)
+ * 7. Do not transfer bundles with namespace isolation policies or anti-affinity group policies.
+ * 8. Limit the max number of brokers to transfer bundle load for each cycle,
+ * (loadBalancerExtentionsTransferShedderMaxNumberOfBrokerTransfersPerCycle).
+ * 9. Print more logs with a debug option(loadBalancerExtentionsTransferShedderDebugModeEnabled=true).
+ */
+public class TransferShedder implements NamespaceUnloadStrategy {
+ private static final Logger log = LoggerFactory.getLogger(TransferShedder.class);
+ private static final double KB = 1024;
+ private final List<Unload> selectedBundlesCache = new ArrayList<>();
+ private final Map<String, Double> brokerAvgResourceUsage = new HashMap<>();
+ private final LoadStats stats = new LoadStats(brokerAvgResourceUsage);
+ private final PulsarService pulsar;
+ private final SimpleResourceAllocationPolicies allocationPolicies;
+
+ @VisibleForTesting
+ public TransferShedder(){
+ this.pulsar = null;
+ this.allocationPolicies = null;
+ }
+
+ public TransferShedder(PulsarService pulsar){
+ this.pulsar = pulsar;
+ this.allocationPolicies = new SimpleResourceAllocationPolicies(pulsar);
+ }
+
+ private static int toPercentage(double usage) {
+ return (int) (usage * 100);
+ }
+
+ @ToString
Review Comment:
```suggestion
@Getter
@Accessors(fluent = true)
```
The `ToString` method already exists. And we can use `@Accessors(fluent = true)` to generate the get method. Then we can use the stats like `stats.minBrokers()`.
--
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