You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "massakam (via GitHub)" <gi...@apache.org> on 2024/04/22 14:20:40 UTC
[PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
massakam opened a new pull request, #22556:
URL: https://github.com/apache/pulsar/pull/22556
Fixes #22442
### Motivation
Please refer to #22442.
### Modifications
- Exclude producers for geo-replication from the `publishers` field in topic-stats and broker-stats.
- Two Pulsar services were started in the `OneWayReplicatorTestBase` class, but they were each using different configuration metadata store instances and were not being replicated. So I made the two Pulsar services use a common configuration metadata store instance and enabled replication.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "massakam (via GitHub)" <gi...@apache.org>.
massakam commented on PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#issuecomment-2071325099
Resolved the conflicts.
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "massakam (via GitHub)" <gi...@apache.org>.
massakam commented on PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#issuecomment-2071211652
@lhotari 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#discussion_r1575054611
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java:
##########
@@ -745,8 +746,7 @@ public void updateRates(NamespaceStats nsStats, NamespaceBundleStats bundleStats
replicators.forEach((region, replicator) -> replicator.updateRates());
- nsStats.producerCount += producers.size();
- bundleStats.producerCount += producers.size();
+ final AtomicInteger producerCount = new AtomicInteger();
Review Comment:
I guess this could be a `org.apache.commons.lang3.mutable.MutableInt` since this is used in single threaded code.
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "nkurihar (via GitHub)" <gi...@apache.org>.
nkurihar merged PR #22556:
URL: https://github.com/apache/pulsar/pull/22556
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#issuecomment-2071211068
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/22556?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `86.66667%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 73.91%. Comparing base [(`bbc6224`)](https://app.codecov.io/gh/apache/pulsar/commit/bbc62245c5ddba1de4b1e7cee4ab49334bc36277?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`442bb30`)](https://app.codecov.io/gh/apache/pulsar/pull/22556?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 177 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/22556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #22556 +/- ##
============================================
+ Coverage 73.57% 73.91% +0.34%
- Complexity 32624 33055 +431
============================================
Files 1877 1885 +8
Lines 139502 140365 +863
Branches 15299 15420 +121
============================================
+ Hits 102638 103753 +1115
+ Misses 28908 28569 -339
- Partials 7956 8043 +87
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/22556/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/22556/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.72% <80.00%> (+2.13%)` | :arrow_up: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/22556/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.56% <40.00%> (+0.24%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/22556/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.20% <86.66%> (+0.36%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files](https://app.codecov.io/gh/apache/pulsar/pull/22556?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...oker/service/nonpersistent/NonPersistentTopic.java](https://app.codecov.io/gh/apache/pulsar/pull/22556?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2Fnonpersistent%2FNonPersistentTopic.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL25vbnBlcnNpc3RlbnQvTm9uUGVyc2lzdGVudFRvcGljLmphdmE=) | `71.21% <85.71%> (+1.75%)` | :arrow_up: |
| [...sar/broker/service/persistent/PersistentTopic.java](https://app.codecov.io/gh/apache/pulsar/pull/22556?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2Fpersistent%2FPersistentTopic.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL3BlcnNpc3RlbnQvUGVyc2lzdGVudFRvcGljLmphdmE=) | `78.36% <87.50%> (-0.10%)` | :arrow_down: |
... and [257 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/22556/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#discussion_r1575054920
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -2107,8 +2108,7 @@ public void updateRates(NamespaceStats nsStats, NamespaceBundleStats bundleStats
replicators.forEach((region, replicator) -> replicator.updateRates());
- nsStats.producerCount += producers.size();
- bundleStats.producerCount += producers.size();
+ final AtomicInteger producerCount = new AtomicInteger();
Review Comment:
I guess this could be a `org.apache.commons.lang3.mutable.MutableInt` since this is used in single threaded code.
--
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
Re: [PR] [improve][broker] Exclude producers for geo-replication from publishers field of topic stats [pulsar]
Posted by "massakam (via GitHub)" <gi...@apache.org>.
massakam commented on code in PR #22556:
URL: https://github.com/apache/pulsar/pull/22556#discussion_r1575095343
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java:
##########
@@ -745,8 +746,7 @@ public void updateRates(NamespaceStats nsStats, NamespaceBundleStats bundleStats
replicators.forEach((region, replicator) -> replicator.updateRates());
- nsStats.producerCount += producers.size();
- bundleStats.producerCount += producers.size();
+ final AtomicInteger producerCount = new AtomicInteger();
Review Comment:
Replaced `AtomicInteger` with `MutableInt`.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -2107,8 +2108,7 @@ public void updateRates(NamespaceStats nsStats, NamespaceBundleStats bundleStats
replicators.forEach((region, replicator) -> replicator.updateRates());
- nsStats.producerCount += producers.size();
- bundleStats.producerCount += producers.size();
+ final AtomicInteger producerCount = new AtomicInteger();
Review Comment:
Replaced `AtomicInteger` with `MutableInt`.
--
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