timescale / tsbs

Time Series Benchmark Suite, a tool for comparing and evaluating databases for time series data
MIT License
1.29k stars 300 forks source link

[fix] fixed cpu-max-all-* wrong influxql group by time clause #110

Closed filipecosta90 closed 4 years ago

filipecosta90 commented 4 years ago

overall description

solves #109 [fix] fixed cpu-max-all-* for influx. [add] added influxql to --print-responses option within query runner

If you guys compare timescale's and influx's results for cpu-max-all-1 and cpu-max-all-8 queries, you will notice that there is a wrong group by time clause on influx ( group by time(1m) should be group by time(1h). The changes in this PR fix the issue and improvement the resultset output for influx ( so that we can easily check this type of error.

current master cpu-max-all-1 ( notice group by minute )

ID 0: {
ID 0:   "results": [
ID 0:     {
ID 0:       "statement_id": 0,
ID 0:       "series": [
ID 0:         {
ID 0:           "name": "cpu",
ID 0:           "columns": [
ID 0:             "time",
ID 0:             "max",
ID 0:             "max_1",
ID 0:             "max_2",
ID 0:             "max_3",
ID 0:             "max_4",
ID 0:             "max_5",
ID 0:             "max_6",
ID 0:             "max_7",
ID 0:             "max_8",
ID 0:             "max_9"
ID 0:           ],
ID 0:           "values": [
ID 0:             [
ID 0:               "2016-01-01T02:16:00Z",
ID 0:               36,
ID 0:               9,
ID 0:               49,
ID 0:               13,
ID 0:               32,
ID 0:               3,
ID 0:               9,
ID 0:               29,
ID 0:               23,
ID 0:               52
ID 0:             ],
ID 0:             [
ID 0:               "2016-01-01T02:17:00Z",
ID 0:               37,
ID 0:               10,
ID 0:               47,
ID 0:               11,
ID 0:               32,
ID 0:               7,
ID 0:               10,
ID 0:               28,
ID 0:               25,
ID 0:               52
ID 0:             ],
ID 0:             [
ID 0:               "2016-01-01T02:18:00Z",
ID 0:               39,
ID 0:               10,
ID 0:               44,
ID 0:               13,
ID 0:               28,
ID 0:               9,
ID 0:               7,
ID 0:               30,
ID 0:               28,
ID 0:               51
ID 0:             ],
(...)

after fix cpu-max-all-1 ( notice group by hour )

{
ID 0:   "influxql": "SELECT max(usage_user),max(usage_system),max(usage_idle),max(usage_nice),max(usage_iowait),max(usage_irq),max(usage_softirq),max(usage_steal),max(usage_guest),max(usage_guest_nice) from cpu where (hostname = 'host_9') and time \u003e= '2016-01-01T02:16:22Z' and time \u003c '2016-01-01T10:16:22Z' group by time(1h)",
ID 0:   "response": {
ID 0:     "results": [
ID 0:       {
ID 0:         "series": [
ID 0:           {
ID 0:             "columns": [
ID 0:               "time",
ID 0:               "max",
ID 0:               "max_1",
ID 0:               "max_2",
ID 0:               "max_3",
ID 0:               "max_4",
ID 0:               "max_5",
ID 0:               "max_6",
ID 0:               "max_7",
ID 0:               "max_8",
ID 0:               "max_9"
ID 0:             ],
ID 0:             "name": "cpu",
ID 0:             "values": [
ID 0:               [
ID 0:                 "2016-01-01T02:00:00Z",
ID 0:                 48,
ID 0:                 11,
ID 0:                 55,
ID 0:                 17,
ID 0:                 46,
ID 0:                 25,
ID 0:                 33,
ID 0:                 44,
ID 0:                 38,
ID 0:                 52
ID 0:               ],
ID 0:               [
ID 0:                 "2016-01-01T03:00:00Z",
ID 0:                 27,
ID 0:                 9,
ID 0:                 84,
ID 0:                 27,
ID 0:                 52,
ID 0:                 35,
ID 0:                 33,
ID 0:                 85,
ID 0:                 40,
ID 0:                 59
ID 0:               ],
(...)
RobAtticus commented 4 years ago

Great catch -- looks like the tests need to be slightly updated.

RobAtticus commented 4 years ago

Ping @filipecosta90 . Happy to merge once tests pass.

filipecosta90 commented 4 years ago

Hi there, sorry for the delay. Will fix still today :)

codecov-io commented 4 years ago

Codecov Report

Merging #110 into master will increase coverage by 0.33%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
+ Coverage   69.26%   69.59%   +0.33%     
==========================================
  Files         103      104       +1     
  Lines        5723     5812      +89     
==========================================
+ Hits         3964     4045      +81     
- Misses       1684     1689       +5     
- Partials       75       78       +3
Impacted Files Coverage Δ
query/http.go 93.93% <100%> (+0.18%) :arrow_up:
...d/tsbs_generate_queries/databases/influx/devops.go 100% <100%> (ø) :arrow_up:
...d/tsbs_generate_queries/databases/influx/common.go 85.71% <100%> (+0.52%) :arrow_up:
internal/inputs/generator_data.go 88.81% <0%> (-1.19%) :arrow_down:
internal/inputs/utils.go 78.26% <0%> (ø) :arrow_up:
cmd/tsbs_generate_data/serialize/akumuli.go 92.85% <0%> (ø)
internal/inputs/generator_queries.go 73.17% <0%> (+0.16%) :arrow_up:

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 1d12206...e755d57. Read the comment docs.

filipecosta90 commented 4 years ago

Ping @filipecosta90 . Happy to merge once tests pass.

@RobAtticus done

RobAtticus commented 4 years ago

Thanks! Merged