Skip to content

fix: prevent Summary quantiles from collapsing to the minimum observation#2316

Open
manduinca wants to merge 1 commit into
prometheus:mainfrom
manduinca:fix/2292-ckms-boundary-epsilon
Open

fix: prevent Summary quantiles from collapsing to the minimum observation#2316
manduinca wants to merge 1 commit into
prometheus:mainfrom
manduinca:fix/2292-ckms-boundary-epsilon

Conversation

@manduinca

Copy link
Copy Markdown

Fixes #2292

When a targeted quantile has 2*epsilon >= 1 - quantile, all reported quantiles could collapse to the minimum observation. The query scan stopped too early: a freshly inserted low-rank sample can carry a delta on the order of n, so the running rank + g + delta exceeded the target far sooner than it should and returned a value near the minimum.

The fix has two parts, matching the diagnosis in the issue. compress() now keeps a merged sample from spanning a quantile's target rank, and the query uses the minimum of the error function over a sample's rank interval [lo, hi] rather than at a single point. To support that I split the single-argument f(r) into f(lo, hi) (the plain error function is f(r, r)), which is numerically identical for the existing call sites.

Added a regression test that reproduces the collapse and asserts distinct quantiles. CKMSQuantilesTest passes (18 tests).

…tion

CKMSQuantiles returned the minimum observation for every targeted quantile
whenever 2*epsilon >= 1-quantile (e.g. quantile(0.9, 0.05) or
quantile(0.99, 0.005)). At the boundary the error function permits a sample's
uncertainty (delta) to reach the order of n at low ranks, which broke both the
query and compression:

- get() stopped at the first sample whose maximum rank exceeded
  desiredRank + f(desiredRank)/2 and returned the preceding sample. A freshly
  inserted low-rank sample (delta = f(r) - 1) then made the scan stop almost
  immediately, returning a value near the minimum. get() now returns the sample
  whose possible-rank interval is centered closest to the desired rank.
- compress() bounded merges by the error function at the left edge only, where
  f is huge for low ranks, so it merged away all resolution between the median
  and the maximum. Merges are now bounded by the error function over the whole
  rank interval the merged sample would span, via a new f(lo, hi) overload
  (f(r) delegates to f(r, r), so single-rank behavior is unchanged).

Adds regression tests for the boundary configuration.

Fixes prometheus#2292

Signed-off-by: Jean Pierre Mandujano G. <jeanpierre.mandujano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Summary quantiles collapse to the minimum observation when 2·epsilon ≥ 1−quantile

1 participant