Skip to content

[fix][broker] Fix precision loss in DataSketchesSummaryLogger by replacing LongAdder with DoubleAdder for sum accumulation#25594

Merged
nodece merged 1 commit into
apache:masterfrom
zjxxzjwang:master-metric-sum-precision-loss
Apr 29, 2026
Merged

[fix][broker] Fix precision loss in DataSketchesSummaryLogger by replacing LongAdder with DoubleAdder for sum accumulation#25594
nodece merged 1 commit into
apache:masterfrom
zjxxzjwang:master-metric-sum-precision-loss

Conversation

@zjxxzjwang
Copy link
Copy Markdown
Contributor

Fixes

Motivation

In DataSketchesSummaryLogger#registerEvent, the valueMillis (a double) was previously cast to long before being added to sumAdder:

This caused precision loss in the _sum metric exposed to Prometheus:

  1. Truncation of fractional parts: Every event's sub-millisecond latency was silently discarded. For example, 1.8ms was recorded as 1ms.
  2. Sub-millisecond latencies become zero: When latency is less than 1ms (e.g., 500 microseconds = 0.5ms), the cast to long produces 0, meaning these events contribute nothing to the sum.
  3. Cumulative error: In high-throughput scenarios, the accumulated truncation error grows continuously, causing the _sum metric to be significantly lower than the actual total latency.

This leads to inaccurate _sum values in Prometheus Summary metrics, which in turn causes incorrect average latency calculations (sum / count).

Modifications

Replace LongAdder with DoubleAdder for sumAdder in DataSketchesSummaryLogger to preserve the full double precision of valueMillis, eliminating the lossy (long) cast.

Verifying this change

This change is a small refactor that fixes a data precision issue. The existing tests should continue to pass. The behavioral change is that getSum() now returns double instead of long, which is compatible with all call sites since they already accept double values (e.g., MetricFamilySamples.Sample constructor and writeMetric methods).

Does this pull request potentially affect one of the following parts?

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc-required
  • doc-not-needed
  • doc
  • doc-complete

…acing LongAdder with DoubleAdder for sum accumulation
Copy link
Copy Markdown
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

@nodece nodece merged commit 00577a5 into apache:master Apr 29, 2026
44 checks passed
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request May 6, 2026
…acing LongAdder with DoubleAdder for sum accumulation (apache#25594)
@lhotari lhotari added this to the 5.0.0-M1 milestone Jun 1, 2026
lhotari pushed a commit that referenced this pull request Jun 1, 2026
…acing LongAdder with DoubleAdder for sum accumulation (#25594)

(cherry picked from commit 00577a5)
lhotari pushed a commit that referenced this pull request Jun 1, 2026
…acing LongAdder with DoubleAdder for sum accumulation (#25594)

(cherry picked from commit 00577a5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants