apache / apache/pulsar

[java client] Cannot integrate ProducerStats with external metrics systems

Open
#10,100 1 comment 1 reaction 0 assignees View on GitHub
lifecycle/stale type/bug
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 22h
Merged PRs (30d)
142

Description

**Describe the bug**
In case of PartitionedProducerImpl the getStats() method modifies the state of the internal "stats" object

```
public synchronized ProducerStatsRecorderImpl getStats() {
if (stats == null) {
return null;
}
stats.reset();
for (int i = 0; i < topicMetadata.numPartitions(); i++) {
stats.updateCumulativeStats(producers.get(i).getStats());
}
return stats;
}
```

The fact that the method is `synchronized` does not save the user from having corrupted stats, because the getStats() returns a reference to the internal `stats` object.
So if you call getStats() and the call some methods on the returned object from two different threads it can happen that the first thread sees corrupted values, because the second thread calls `reset` in the meantime.

```
ProducerStats stats = producer.getStats();
// accessing an object that is mutated from a different thread
long totalMsgsSent = stats.getTotalMsgsSent();
```

This behaviour prevents you from collecting the stats using an external system that does not provide a documented and consistent behaviour about concurrency.

For instance I would like to create "Gauges" and attach them to each of the exposed metrics, but it is actually not possible.
I can do it with this kind of code, by it is likely to break if we change the implementation of the PulsarProducer

```
Gauge gauge = () -> {
synchronized(producer) {
ProducerStats stats = producer.getStats();
long totalMsgsSent = stats.getTotalMsgsSent();
return totalMsgsSent;
}}
```

* Suggestions for improvement
Create a new instance of ProducerStats in PartitionedProducerImpl instead of recycling the existing object

Contributor guide

Open the contributing guide

Research direction

Start in PartitionedProducerImpl#getStats and inspect how its internal stats object is reset and populated from the partition producers. Reproduce concurrent reads of the returned ProducerStats, then add coverage for the issue's external-metrics use case; done means callers do not observe stats being mutated by a later getStats call.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend-api-design, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.