spotify / heroic

The Heroic Time Series Database
https://spotify.github.io/heroic/
Apache License 2.0
848 stars 109 forks source link

Add a simple group combiners and additional IT tests. #744

Closed ao2017 closed 3 years ago

ao2017 commented 3 years ago
  1. Add a new group combiner to handle simple tdigest query This combiner handles query that doesn't include aggregation clause. Query will return the record count in each distributionPoint instead of a binary data.

  2. Refactor DistributedCombiner.

  3. Add integration test to validate TdigestBucket thread safety

  4. Add addition integration test for tdigest

  5. Remove lock free implementation of TdigestBucket .

Sample Query :

  1. curl --location --request POST ''http://localhost:8081/query/metrics'' --header 'Content-Type: application/json' --data-raw '{ "source":"distributionPoints", "range": {"type": "relative", "unit": "DAYS", "value": 30}, "filter": ["and", ["=", "run", "17716d49eab"], ["=", "metric_type", "distribution"]] }' return datapoint count in each distributionPoint and -1 if batch is empty.

  2. curl --location --request POST 'http://localhost:8081/query/metrics' --header 'Content-Type: application/json' --data-raw '{ "source":"distributionPoints","features":["com.spotify.heroic.distributed_aggregations"], "range": {"type": "relative", "unit": "DAYS", "value": 30}, "filter": ["and", ["=", "run", "17716d49eab"], ["=", "metric_type", "distribution"]], "aggregation": { "type": "group", "of": ["site"], "each": { "type": "tdigest" }} }' merge and compute default percentile ( P99, p50 and p75)

lmuhlha commented 3 years ago

just a few questions but lgtm

codecov[bot] commented 3 years ago

Codecov Report

Merging #744 (7fb4874) into master (1b45699) will increase coverage by 0.05%. The diff coverage is 84.55%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #744      +/-   ##
============================================
+ Coverage     55.03%   55.09%   +0.05%     
- Complexity     3131     3146      +15     
============================================
  Files           743      746       +3     
  Lines         20248    20334      +86     
  Branches       1316     1329      +13     
============================================
+ Hits          11144    11203      +59     
- Misses         8632     8651      +19     
- Partials        472      480       +8     
Impacted Files Coverage Δ Complexity Δ
.../com/spotify/heroic/aggregation/EmptyInstance.java 71.42% <0.00%> (-1.75%) 5.00 <0.00> (ø)
...a/com/spotify/heroic/metric/DistributionPoint.java 40.00% <ø> (ø) 3.00 <0.00> (ø)
...va/com/spotify/heroic/metric/MetricCollection.java 65.51% <0.00%> (-1.15%) 12.00 <0.00> (ø)
...y/heroic/profile/ElasticsearchMetadataProfile.java 10.34% <0.00%> (-1.66%) 2.00 <0.00> (ø)
...potify/heroic/aggregation/AggregationCombiner.java 80.00% <66.66%> (-16.78%) 1.00 <0.00> (-7.00)
.../heroic/aggregation/simple/TdigestMergingBucket.kt 78.57% <72.72%> (-21.43%) 5.00 <3.00> (+1.00) :arrow_down:
...fy/heroic/metric/DistributionPointDeserialize.java 84.61% <84.61%> (ø) 3.00 <3.00> (?)
...heroic/aggregation/TDigestAggregationCombiner.java 92.42% <92.42%> (ø) 18.00 <18.00> (?)
...otify/heroic/aggregation/simple/TdigestInstance.kt 100.00% <100.00%> (ø) 6.00 <2.00> (ø)
...eroic/aggregation/simple/TdigestInstanceUtils.java 9.09% <100.00%> (-80.91%) 1.00 <1.00> (-4.00)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e18fc9f...7fb4874. Read the comment docs.