apache / apache/beam

HistogramCell needs a few touches to make it match the spec :)

Open
#20,853 0 comments 0 reactions 0 assignees View on GitHub
fn-execution java P3 tests
Dominant language
Java
Stars
8.7k
Forks
4.7k
Avg merge
1d 20h
Merged PRs (30d)
196

Description

See spec here:

 __ [_https://s.apache.org/beam-histogram-metrics_](_https://s.apache.org/beam-histogram-metrics_)

A few pieces I want to highlight:

  *//   LINEAR(start, width, num_buckets):*

  *//       Bi = start **** width * i*

  *//       e.g. LINEAR(5, 15, 6) defines:*

  *//           [-INF, 5), [5, 20), [20, 35), [35, 50), [50, 65), [65, ****INF], ...*

Current Java implementation:

LinearBuckets.of(5, 15, 6) will defined buckets like this (6 buckets excluding the infinite ranges)

[-INF, 5), [5, 20), [20, 35), [35, 50), [50, 65), [65, 80), [80, 95), [95, ****INF], ...

 

Additionally, we should treat the infinite buckets like any other, i.e. as a counter in the array, rather than having separate variables for the -inf and inf buckets.:

i.e. we can remove numTopRecords numBottomRecords

public HistogramData(BucketType bucketType)

{ this.bucketType = bucketType; this.buckets = new long[bucketType.getNumBuckets()]; this.numOfRecords = 0; this.numTopRecords = 0; this.numBottomRecords = 0; }

 

Additionally getCount() and incBucketCount() should use those same indexes

i.e. for all these functions 0 means the -INF bucket, and length - 1 means the INF bucket

This code is only used for one internal metric, so this we should change this before it gets adopted in other places. 

 

Imported from Jira [BEAM-12103](https://issues.apache.org/jira/browse/BEAM-12103). Original Jira may contain additional context.
Reported by: ajamato@google.com.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.