You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/06/22 10:37:12 UTC
[GitHub] [pulsar-manager] tuteng opened a new pull request #306: Support multi address for broker stats
tuteng opened a new pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306
Master Issue: #<xyz>
### Motivation
When the cluster is configured with multiple service addresses, the web service URL will be parsed incorrectly
### Modifications
* Support multi-address for broker stats collect metrics.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] tuteng commented on pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
tuteng commented on pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306#issuecomment-648672589
ping @eolivelli PTAL
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] eolivelli commented on a change in pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306#discussion_r444737972
##########
File path: src/main/java/org/apache/pulsar/manager/service/impl/BrokerStatsServiceImpl.java
##########
@@ -131,7 +132,26 @@ private void scheduleCollectStats() {
clusterLists.forEach((clusterMap) -> {
String cluster = (String) clusterMap.get("cluster");
Pair<String, String> envCluster = Pair.of(env.getName(), cluster);
- collectStatsServiceUrls.put(envCluster, (String) clusterMap.get("serviceUrl"));
+ String webServiceUrl = (String) clusterMap.get("serviceUrl");
+ if (webServiceUrl.contains(",")) {
+ String[] webServiceUrlList = webServiceUrl.split(",");
+ if (StringUtils.isNotBlank(pulsarJwtToken)) {
+ header.put("Authorization", String.format("Bearer %s", pulsarJwtToken));
+ }
+ for (String url : webServiceUrlList) {
+ if (!url.contains("http://")) {
+ url = "http://" + url;
+ }
+ String httpTestResult = HttpUtil.doGet( url + "/metrics", header);
Review comment:
Sorry for late reply.
It looks like we are going to call metrics endpoint twice.
Isn't there any way to download such data only once?
Like pushing down the list of endpoints to downstream processing?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] tuteng commented on a change in pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
tuteng commented on a change in pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306#discussion_r444873729
##########
File path: src/main/java/org/apache/pulsar/manager/service/impl/BrokerStatsServiceImpl.java
##########
@@ -131,7 +132,26 @@ private void scheduleCollectStats() {
clusterLists.forEach((clusterMap) -> {
String cluster = (String) clusterMap.get("cluster");
Pair<String, String> envCluster = Pair.of(env.getName(), cluster);
- collectStatsServiceUrls.put(envCluster, (String) clusterMap.get("serviceUrl"));
+ String webServiceUrl = (String) clusterMap.get("serviceUrl");
+ if (webServiceUrl.contains(",")) {
+ String[] webServiceUrlList = webServiceUrl.split(",");
+ if (StringUtils.isNotBlank(pulsarJwtToken)) {
+ header.put("Authorization", String.format("Bearer %s", pulsarJwtToken));
+ }
+ for (String url : webServiceUrlList) {
+ if (!url.contains("http://")) {
+ url = "http://" + url;
+ }
+ String httpTestResult = HttpUtil.doGet( url + "/metrics", header);
Review comment:
@eolivelli I changed the interface to call health check, but it still needs several HTTP calls. I haven't found a way to get all the broker status at once. Now, this call is in a scheduled task, and it will run every 5 minutes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] eolivelli commented on a change in pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306#discussion_r443469051
##########
File path: src/main/java/org/apache/pulsar/manager/service/impl/BrokerStatsServiceImpl.java
##########
@@ -131,7 +131,12 @@ private void scheduleCollectStats() {
clusterLists.forEach((clusterMap) -> {
String cluster = (String) clusterMap.get("cluster");
Pair<String, String> envCluster = Pair.of(env.getName(), cluster);
- collectStatsServiceUrls.put(envCluster, (String) clusterMap.get("serviceUrl"));
+ String webServiceUrl = (String) clusterMap.get("serviceUrl");
+ if (webServiceUrl.contains(",")) {
+ String[] webServiceUrlList = webServiceUrl.split(",");
+ webServiceUrl = webServiceUrlList[0];
Review comment:
here you are keeping only the first one.
what about if it is down ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] wolfstudy merged pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
wolfstudy merged pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-manager] tuteng commented on a change in pull request #306: Support multi address for broker stats
Posted by GitBox <gi...@apache.org>.
tuteng commented on a change in pull request #306:
URL: https://github.com/apache/pulsar-manager/pull/306#discussion_r443485482
##########
File path: src/main/java/org/apache/pulsar/manager/service/impl/BrokerStatsServiceImpl.java
##########
@@ -131,7 +131,12 @@ private void scheduleCollectStats() {
clusterLists.forEach((clusterMap) -> {
String cluster = (String) clusterMap.get("cluster");
Pair<String, String> envCluster = Pair.of(env.getName(), cluster);
- collectStatsServiceUrls.put(envCluster, (String) clusterMap.get("serviceUrl"));
+ String webServiceUrl = (String) clusterMap.get("serviceUrl");
+ if (webServiceUrl.contains(",")) {
+ String[] webServiceUrlList = webServiceUrl.split(",");
+ webServiceUrl = webServiceUrlList[0];
Review comment:
Thanks, @eolivelli, I fixed it, PTAL
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org