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/15 11:09:51 UTC
[GitHub] [pulsar] equanz commented on a diff in pull request #18807: [fix][monitor] Fix a partitioned publisher topic stat aggregation bug
equanz commented on code in PR #18807:
URL: https://github.com/apache/pulsar/pull/18807#discussion_r1049476590
##########
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java:
##########
@@ -252,19 +253,15 @@ public TopicStatsImpl add(TopicStats ts) {
return newStats;
}).add((PublisherStatsImpl) s);
} else {
- if (this.publishers.size() != stats.publishers.size()) {
- for (int i = 0; i < stats.publishers.size(); i++) {
- PublisherStatsImpl newStats = new PublisherStatsImpl();
- newStats.setSupportsPartialProducer(false);
- this.publishers.add(newStats.add(stats.publishers.get(i)));
- }
- } else {
- for (int i = 0; i < stats.publishers.size(); i++) {
- this.publishers.get(i).add(stats.publishers.get(i));
- }
+ if (index == this.publishers.size()) {
+ PublisherStatsImpl newStats = new PublisherStatsImpl();
+ newStats.setSupportsPartialProducer(false);
+ this.publishers.add(newStats);
}
+ this.publishers.get(index)
+ .add((PublisherStatsImpl) s);
Review Comment:
nits:
The `publishers` field is not thread-safe from before. Potentially access to it from concurrent threads.
Moreover, it doesn't guarantee the size is `index + 1` in L262 because it isn't held mutual lock between L259 and L262.
https://github.com/apache/pulsar/blob/1107af24932e5703209f1e1e896953328ce3ddaa/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java#L111
https://github.com/apache/pulsar/blob/1107af24932e5703209f1e1e896953328ce3ddaa/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java#L179
We want to avoid IndexOutOfBoundsException completely.
--
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