From 19560487d4448ff4d8434719cc5968921bfbb525 Mon Sep 17 00:00:00 2001 From: Diana Corbacho Date: Tue, 21 Feb 2017 16:31:40 +0000 Subject: [PATCH] Avoid filling towards the past when we have older values available --- .../src/exometer_slide.erl | 8 +++- .../test/exometer_slide_SUITE.erl | 42 ++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/deps/rabbitmq_management_agent/src/exometer_slide.erl b/deps/rabbitmq_management_agent/src/exometer_slide.erl index 00c8fb79a5..f17a8c94b0 100644 --- a/deps/rabbitmq_management_agent/src/exometer_slide.erl +++ b/deps/rabbitmq_management_agent/src/exometer_slide.erl @@ -66,6 +66,9 @@ sum/5, optimize/1]). +%% For testing +-export([buffer/1]). + -compile(inline). -compile(inline_list_funcs). @@ -461,7 +464,8 @@ take_since([{DropTS, drop} | T], Now, Start, N, [{TS, Evt} | _] = Acc, -Interval, N)], {undefined, lists:reverse(Fill) ++ Acc}; [{TS0, _} = E | Rest] when TS0 >= Start, N > 0 -> - Fill = [{TS1, Evt} || TS1 <- truncated_seq(TS0 + Interval, TS - Interval, + Fill = [{TS1, Evt} || TS1 <- truncated_seq(TS0 + Interval, + max(TS0 + Interval, TS - Interval), Interval, N)], take_since(Rest, Now, Start, decr(N), [E | Fill ++ Acc], Interval); [Prev | _] -> % next sample is out of range so needs to be filled from Start @@ -502,3 +506,5 @@ map_timestamp(TS, Start, Interval, Round) -> Factor = Round((TS - Start) / Interval), Start + Interval * Factor. +buffer(#slide{buf1 = Buf1, buf2 = Buf2}) -> + Buf1 ++ Buf2. diff --git a/deps/rabbitmq_management_agent/test/exometer_slide_SUITE.erl b/deps/rabbitmq_management_agent/test/exometer_slide_SUITE.erl index eb8734e6b4..77e5aff5bd 100644 --- a/deps/rabbitmq_management_agent/test/exometer_slide_SUITE.erl +++ b/deps/rabbitmq_management_agent/test/exometer_slide_SUITE.erl @@ -47,7 +47,8 @@ groups() -> to_normalized_list, to_normalized_list_no_padding, to_list_in_the_past, - sum_mgmt_352 + sum_mgmt_352, + sum_mgmt_352_extra ]} ]. @@ -500,13 +501,31 @@ to_list_in_the_past(_Config) -> [] = exometer_slide:to_list(50, 10, S). sum_mgmt_352(_Config) -> + %% In bug mgmt#352 all the samples returned have the same vale Slide = sum_mgmt_352_slide(), Last = 1487689330000, First = 1487689270000, Incr = 5000, Empty = {0}, Sum = exometer_slide:sum(Last, First, Incr, [Slide], Empty), - ct:pal("SUM is ~p~n", [Sum]), + Values = sets:to_list(sets:from_list( + [V || {_, V} <- exometer_slide:buffer(Sum)])), + true = (length(Values) == 12), + ok. + +sum_mgmt_352_extra(_Config) -> + %% Testing previous case clause to the one that fixes mgmt_352 + %% exometer_slide.erl#L463 + %% In the buggy version, all but the last sample are the same + Slide = sum_mgmt_352_slide_extra(), + Last = 1487689330000, + First = 1487689260000, + Incr = 5000, + Empty = {0}, + Sum = exometer_slide:sum(Last, First, Incr, [Slide], Empty), + Values = sets:to_list(sets:from_list( + [V || {_, V} <- exometer_slide:buffer(Sum)])), + true = (length(Values) == 13), ok. %% ------------------------------------------------------------------- @@ -537,3 +556,22 @@ sum_mgmt_352_slide() -> {1487689257486,{1205600}}], [], {1591000}}. + +sum_mgmt_352_slide_extra() -> + {slide,610000,45,122,true,5000,1487689328468,1487689106834, + [{1487689328468,{1574200}}, + {1487689323467,{1538800}}, + {1487689318466,{1500800}}, + {1487689313465,{1459138}}, + {1487689308463,{1419200}}, + {1487689303462,{1379600}}, + {1487689298461,{1340000}}, + {1487689293460,{1303400}}, + {1487689288460,{1265600}}, + {1487689283458,{1231400}}, + {1487689278457,{1215800}}, + {1487689273456,{1215200}}, + {1487689272487,drop}, + {1487689269486,{1205600}}], + [], + {1591000}}.