samplingInterval splits rollup groups for sFlow rows the correction never reads it for

Open
  • Is something wrong?
#608 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Needs clarification
Activity status
Active
Tech stack
clickhouse, java
Domain
data-engineering, databases

Research direction

Start by reading the context in #470, #583, and #606, then measure sFlow-only cardinality in the by_application and by_geo_asn rollups. Compare the rows attributable only to differing sampling rates and document whether to accept the current behavior or change the stored rollup dimension. Done means the trade-off is measured and one option is selected before another dimension is appended.

Written by the indexing model from the issue text.

Description

enhancement java priority: low

Deferred from the review of #606 (closes #583).

#470 made samplingInterval a sort-key dimension on all four rollups so sampling-corrected volume survives the raw table's retention. #606 then established that the correction must never apply that rate to an sFlow row, because sFlow scales its counters at ingest:

sum(bytes * if(flowProtocol = 'SFLOW', 1, samplingInterval))

For every sFlow row, the if() resolves to 1 and samplingInterval is not read. The column still sits in the sorting key, so it still splits groups: two sFlow exporters sampling at different rates produce two rollup rows where one would do, and a rate that drifts over time keeps splitting the same group. On an sFlow-only deployment the rate contributes to no corrected total anywhere and costs cardinality everywhere.

#606 reasoned carefully about the cost of the column it added and not at all about the column it made inert.

Scale

Unmeasured. The relevant figure is what fraction of rollup rows on an sFlow-heavy deployment exist only because two rates landed in one group — a measurement much like #470's 1.0214 median, restricted to sFlow traffic. The flows_by_exporter_iface_1m rollup is already keyed by exporter so the split there is inherent, but by_application and by_geo_asn group across exporters and are exposed.

Why this is not obviously a bug

Riptide records what the exporter reported (#467). An sFlow agent's sampling rate is a real statement about how it sampled, useful for diagnostics and for samplingProvenance, and discarding it to save sort-key width would be the option #583 explicitly rejected for the rollups. There is a genuine tension between "record faithfully" and "do not pay cardinality for a value no query reads".

Options worth weighing

  1. Accept and document. Cheapest. Say plainly in receivers.md that the rate is carried for sFlow rows even though the correction ignores it.
  2. Fold the policy into the stored value. Carry if(f.flowProtocol = 'SFLOW', 1, f.samplingInterval) as the rollup dimension instead of the raw rate. This is the "residual factor" option considered and rejected in #583's design — rejected on generality, not on cost, and this issue is the cost argument it did not have at the time. Note it cannot replace samplingInterval in place without a second boundary.
  3. Measure first and decide after. Probably right: the answer depends entirely on a number nobody has.

Blocked on nothing. Worth doing before another dimension is appended, since each append compounds the sort key.

Dominant language
Java
Stars
2
Forks
0
Avg merge
1d 1h
Merged PRs (30d)
52

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Riptide-Labs/riptide

All issues in Riptide-Labs/riptide

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.