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/12/01 20:36:48 UTC
[GitHub] [pulsar] rdhabalia opened a new pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
rdhabalia opened a new pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074
### Motivation
There are a few load balancer config that requires tuning at runtime without restarting the broker. so, mark those configs dynamic so, admin can tune them based on cluster requirements.
--
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] rdhabalia commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-1007859399
ping. can someone please review the PR?
--
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] rdhabalia commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r761283650
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
done
--
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] github-actions[bot] commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-984034277
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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] Jason918 commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r761547685
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
Only loadBalancerLoadSheddingStrategy needs restart? Have you checked others?
--
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] rdhabalia commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r760798828
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
we would like to change config without deploying new code and dynamic config helps in that case. for a safe deployment we would like keep existing config, change config dynamically and do process restart without redeploying new code. so, this is an intentional code change.
--
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] rdhabalia commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-984811062
@eolivelli dynamic config provides a way to change broker-config value dynamically using admin-api and it also persists those changed values and applies them on broker server startup. this feature is useful when you want to tune broker at runtime or experiment any value without deploying new release of changes.
--
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] rdhabalia commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r761270968
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
yes, we can add into doc. I will add it.
--
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] rdhabalia commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-998208597
ping.
--
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] eolivelli merged pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074
--
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] Jason918 commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-998556469
> change config without deploying new code
I think it's a common require and we can add this as default feature for most of the configs.
As for `dynamic`, IMHO, it's better to keep it to annotate the configs that can be update and take effect online.
--
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] rdhabalia commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-999139229
I think we kept most of them dynamic which are required.
However, can we merge this PR as it's been open for long and time require to tune load balancer.
--
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] Jason918 commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r760714413
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
It seems this `loadBalancerLoadSheddingStrategy` is used on broker start, dynamic won't take effect.
Please check if all these configs are only used on broker start up.
--
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] Jason918 commented on a change in pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#discussion_r760994210
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1722,6 +1723,7 @@
@FieldContext(
category = CATEGORY_LOAD_BALANCER,
+ dynamic = true,
doc = "load balance load shedding strategy"
)
private String loadBalancerLoadSheddingStrategy = "org.apache.pulsar.broker.loadbalance.impl.OverloadShedder";
Review comment:
I get your point. But is a little confusing, if we just mark it dynamic, but we need to restart to make it take effect.
Most importantly, we need to tell the difference that which of these dynamic configs need broker restart, and which ones don't.
--
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] eolivelli commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-984395669
@rdhabalia can you please give me some pointers about how "dynamic" works ?
--
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] rdhabalia commented on pull request #13074: [pulsar-broker] Make load-balancer config dynamic for the runtime tuning
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #13074:
URL: https://github.com/apache/pulsar/pull/13074#issuecomment-1007859399
ping. can someone please review the PR?
--
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