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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
