void-ptr974 commented on code in PR #25990:
URL: https://github.com/apache/pulsar/pull/25990#discussion_r3400074965
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java:
##########
@@ -1464,20 +1463,15 @@ public PersistentTopic getTopic() {
}
- public synchronized long getDelayedTrackerMemoryUsage() {
+ public long getDelayedTrackerMemoryUsage() {
return
delayedDeliveryTracker.map(DelayedDeliveryTracker::getBufferMemoryUsage).orElse(0L);
}
- public synchronized Map<String, TopicMetricBean>
getBucketDelayedIndexStats() {
- if (delayedDeliveryTracker.isEmpty()) {
- return Collections.emptyMap();
- }
-
- if (delayedDeliveryTracker.get() instanceof
BucketDelayedDeliveryTracker) {
- return ((BucketDelayedDeliveryTracker)
delayedDeliveryTracker.get()).genTopicMetricMap();
- }
-
- return Collections.emptyMap();
+ public Map<String, TopicMetricBean> getBucketDelayedIndexStats() {
+ return delayedDeliveryTracker
+ .filter(BucketDelayedDeliveryTracker.class::isInstance)
+ .map(tracker -> ((BucketDelayedDeliveryTracker)
tracker).genTopicMetricMap())
Review Comment:
Thanks for the update. I think this still needs serialization for
correctness. This method is now reachable from an unsynchronized stats path,
but it is not a pure read: it updates shared `stats`, calls
`stats.genTopicMetricMap()` which drains counters with `sumThenReset()` /
`StatsBuckets.refresh()`, and also reads `sharedBucketPriorityQueue.size()` /
`lastMutableBucket.size()` from non-thread-safe queues. Concurrent stats
scrapes can therefore race with each other and export incorrect samples. The
minimal fix would be to keep this collection serialized, e.g. make
`genTopicMetricMap()` synchronized; if we want it lock-free, the method needs
to become a pure snapshot read from cached/atomic values.
--
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]
