Skip to content

Commit

Permalink
MB-54396 [BP] Make sure promQL optimizations don't change...
Browse files Browse the repository at this point in the history 
... the order of 'or' operands

Change-Id: I82cc84dff7972bf027ee9f202ce3e7879b9876c3
Reviewed-on: https://review.couchbase.org/c/ns_server/+/182229
Well-Formed: Build Bot <build@couchbase.com>
Well-Formed: Restriction Checker
Reviewed-by: Timofey Barmin <timofey.barmin@couchbase.com>
Tested-by: Steve Watanabe <steve.watanabe@couchbase.com>
  • Loading branch information
@stevewatanabe
stevewatanabe committed Nov 30, 2022
1 parent 18850d6 commit 38831e5
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 71 deletions.
4 changes: 2 additions & 2 deletions src/derived_metrics.erl
View file
Expand Up @@ -117,8 +117,8 @@ get_metric(<<"xdcr_percent_completeness">>) ->
percent(_(<<"xdcr_docs_processed_total">>),
fun (M) ->
promQL:sum_without([<<"name">>],
{'or', [M(<<"xdcr_docs_processed_total">>),
M(<<"xdcr_changes_left_total">>)]})
{union, [M(<<"xdcr_docs_processed_total">>),
M(<<"xdcr_changes_left_total">>)]})
end, 100);
get_metric(<<"eventing_processed_count">>) ->
sum([<<"eventing_timer_callback_success">>,
Expand Down
6 changes: 3 additions & 3 deletions src/menelaus_web_stats.erl
View file
Expand Up @@ -930,7 +930,7 @@ construct_promql_query(Labels, Functions, Window, PermFilters) ->
_ -> lists:map(fun (Ast) -> range_vector(Ast, Window) end, _)
end,
lists:map(fun (Ast) -> apply_functions(Ast, RangeVFunctions) end, _),
{'or', _},
{union, _},
apply_functions(_, InstantVFunctions),
promQL:format_promql(_)]).
Expand Down Expand Up @@ -967,7 +967,7 @@ derived_metric_query(Labels, Functions, Window, AuthorizationLabelsList) ->
|| {N, AST} <- maps:to_list(ParamAsts)]
end, AuthorizationLabelsList),
promQL:format_promql({'or', Subqueries}).
promQL:format_promql({union, Subqueries}).
-ifdef(TEST).
Expand Down Expand Up @@ -1143,7 +1143,7 @@ promql_filters_for_roles_test() ->
Filters = fun (Roles) ->
AST = promql_filters_for_roles(Roles, CompiledRoles(Roles)),
promQL:format_promql({'or', AST})
promQL:format_promql({union, AST})
end,
meck:new(cluster_compat_mode, [passthrough]),
Expand Down
119 changes: 62 additions & 57 deletions src/promQL.erl
View file
Expand Up @@ -95,7 +95,8 @@ format_promql(AST) ->
-define(BINOP(Op), Op =:= 'or'; Op =:= 'and'; Op =:= 'unless'; Op =:= '/';
Op =:= '+'; Op =:= '-'; Op =:= '/'; Op =:= '*';
Op =:= '%'; Op =:= '^'; Op =:= '=='; Op =:= '!=';
Op =:= '>'; Op =:= '<'; Op =:= '>='; Op =:= '<=').
Op =:= '>'; Op =:= '<'; Op =:= '>='; Op =:= '<=';
Op =:= union).

-define(AGGREGATION_OP(Op), Op =:= sum; Op =:= min; Op =:= max;
Op =:= avg; Op =:= group; Op =:= stddev;
Expand All @@ -119,11 +120,13 @@ format_promql_ast({call, F, By, Args}) ->
[F, ByStr, "(", lists:join(",", [format_promql_ast(E) || E <- Args]), ")"];
format_promql_ast({Op, Exprs}) when ?BINOP(Op) ->
format_promql_ast({Op, [], Exprs});
format_promql_ast({Op, Opts, Exprs0}) when ?BINOP(Op) ->
Exprs = case Op of
'or' -> merge_or_operands(Exprs0);
_ -> Exprs0
end,
format_promql_ast({union, Opts, Exprs}) ->
%% We treat it as 'or' but we assume that operands don't intersect in this
%% case and we can change their order (it is not true for 'or' operands in
%% general). If we have those properties we can try to apply some
%% optimizations.
format_promql_ast({'or', Opts, merge_union_operands(Exprs)});
format_promql_ast({Op, Opts, Exprs}) when ?BINOP(Op) ->
OptsIOList = lists:map(fun ({T, L}) ->
[atom_to_list(T), "(", lists:join(",", L), ") "]
end, Opts),
Expand Down Expand Up @@ -168,32 +171,32 @@ format_promql_ast(X) when is_float(X) ->
%% Transform "f({name=`m1`, ...}) or f({name=`m2`, ...} or ..." to
%% "f({name=~`m1|m1|...`, ...})" as it works faster.
%% Note: it's correct only if function 'f' commutes with 'or'
merge_or_operands(List) ->
merge_union_operands(List) ->
Sorted = lists:usort(
fun (A, B) ->
comparable(A) =< comparable(B)
end, List),
merge_or_operands_sorted(Sorted, []).

merge_or_operands_sorted([], Res) -> lists:reverse(Res);
merge_or_operands_sorted([E], Res) -> merge_or_operands_sorted([], [E | Res]);
merge_or_operands_sorted([E1, E2 | T], Res) ->
case merge_or_operands(E1, E2) of
match -> merge_or_operands_sorted([E1 | T], Res);
{merged, E} -> merge_or_operands_sorted([E | T], Res);
conflict -> merge_or_operands_sorted([E2 | T], [E1 | Res])
merge_union_operands_sorted(Sorted, []).

merge_union_operands_sorted([], Res) -> lists:reverse(Res);
merge_union_operands_sorted([E], Res) -> merge_union_operands_sorted([], [E | Res]);
merge_union_operands_sorted([E1, E2 | T], Res) ->
case merge_union_operands(E1, E2) of
match -> merge_union_operands_sorted([E1 | T], Res);
{merged, E} -> merge_union_operands_sorted([E | T], Res);
conflict -> merge_union_operands_sorted([E2 | T], [E1 | Res])
end.

merge_or_operands({Op, [Op1, Scalar]}, {Op, [Op2, Scalar]})
merge_union_operands({Op, [Op1, Scalar]}, {Op, [Op2, Scalar]})
when (Op =:= '*' orelse
Op =:= '/'),
is_number(Scalar) ->
case merge_or_operands(Op1, Op2) of
case merge_union_operands(Op1, Op2) of
match -> match;
conflict -> conflict;
{merged, M} -> {merged, {Op, [M, Scalar]}}
end;
merge_or_operands({call, F, By, Args1}, {call, F, By, Args2})
merge_union_operands({call, F, By, Args1}, {call, F, By, Args2})
when length(Args1) == length(Args2) ->
case commute_with_or(F, By) of
true ->
Expand All @@ -202,13 +205,13 @@ merge_or_operands({call, F, By, Args1}, {call, F, By, Args2})
fun ({_, _}, conflict) ->
{undefined, conflict};
({A1, A2}, merged) ->
case merge_or_operands(A1, A2) of
case merge_union_operands(A1, A2) of
conflict -> {undefined, conflict};
match -> {A1, merged};
{merged, _} -> {undefined, conflict}
end;
({A1, A2}, match) ->
case merge_or_operands(A1, A2) of
case merge_union_operands(A1, A2) of
conflict -> {undefined, conflict};
match -> {A1, match};
{merged, M} -> {M, merged}
Expand All @@ -222,13 +225,13 @@ merge_or_operands({call, F, By, Args1}, {call, F, By, Args2})
false ->
conflict
end;
merge_or_operands({range_vector, E1, D}, {range_vector, E2, D}) ->
case merge_or_operands(E1, E2) of
merge_union_operands({range_vector, E1, D}, {range_vector, E2, D}) ->
case merge_union_operands(E1, E2) of
match -> match;
conflict -> conflict;
{merged, E} -> {merged, {range_vector, E, D}}
end;
merge_or_operands({L1}, {L2}) when is_list(L1), is_list(L2) ->
merge_union_operands({L1}, {L2}) when is_list(L1), is_list(L2) ->
case {extract_merge_label(L1), extract_merge_label(L2)} of
{{Names, Rest}, {Names, Rest}} ->
match;
Expand All @@ -238,8 +241,8 @@ merge_or_operands({L1}, {L2}) when is_list(L1), is_list(L2) ->
_ ->
conflict
end;
merge_or_operands(Q, Q) -> match;
merge_or_operands(_, _) -> conflict.
merge_union_operands(Q, Q) -> match;
merge_union_operands(_, _) -> conflict.

comparable({List}) when is_list(List) ->
case extract_merge_label(List) of
Expand Down Expand Up @@ -375,59 +378,61 @@ format_promql_test() ->
{'+', [{[{eq, <<"l3">>, <<"v3">>}]},
{[{eq, <<"l4">>, <<"v4">>}]}]}]}),
<<"({l1=`v1`} + {l2=`v2`}) * ({l3=`v3`} + {l4=`v4`})">>),
?assertEqual(format_promql({'or', [{[{eq, <<"l1">>, <<"v1">>}]},
{[{eq, <<"l1">>, <<"v1">>}]}]}),
?assertEqual(format_promql({[{not_any, <<"name">>,
[<<"v1">>, <<"v2">>, <<"v3">>]}]}),
<<"{name!~`v1|v2|v3`}">>).

merge_test() ->
?assertEqual(format_promql({union, [{[{eq, <<"l1">>, <<"v1">>}]},
{[{eq, <<"l1">>, <<"v1">>}]}]}),
<<"{l1=`v1`}">>),
?assertEqual(format_promql({'or', [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>}]}]}),
?assertEqual(format_promql({union, [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>}]}]}),
<<"{name=~`v1|v2`}">>),
?assertEqual(format_promql({'or', [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>},
{eq, <<"l2">>, <<"v3">>}]}]}),
?assertEqual(format_promql({union, [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>},
{eq, <<"l2">>, <<"v3">>}]}]}),
<<"{name=`v1`} or {name=`v2`,l2=`v3`}">>),
?assertEqual(format_promql(
{'or', [{call, irate, none,
[{range_vector,
{[{eq, <<"name">>, <<"v1">>},
{eq, <<"l1">>, <<"v2">>}]},
<<"1m">>}]},
{[{eq, <<"name">>, <<"v2">>},
{eq, <<"l1">>, <<"v2">>}]},
{call, irate, none,
[{range_vector,
{[{eq, <<"l1">>, <<"v2">>},
{eq_any, <<"name">>, [<<"v2">>,<<"v3">>]}]},
<<"1m">>}]}]}),
{union, [{call, irate, none,
[{range_vector,
{[{eq, <<"name">>, <<"v1">>},
{eq, <<"l1">>, <<"v2">>}]},
<<"1m">>}]},
{[{eq, <<"name">>, <<"v2">>},
{eq, <<"l1">>, <<"v2">>}]},
{call, irate, none,
[{range_vector,
{[{eq, <<"l1">>, <<"v2">>},
{eq_any, <<"name">>, [<<"v2">>,<<"v3">>]}]},
<<"1m">>}]}]}),
<<"{name=`v2`,l1=`v2`} or "
"irate({name=~`v1|v2|v3`,l1=`v2`}[1m])">>),
?assertEqual(format_promql(
{'or',
{union,
[{call, f, none, [1, {[{eq, <<"name">>, <<"v1">>}]}, 2]},
{call, f, none, [1, {[{eq, <<"name">>, <<"v1">>}]}, 2]}]}),
<<"f(1,{name=`v1`},2)">>),
?assertEqual(format_promql(
{'or',
{union,
[{call, f, none, [1, {[{eq, <<"name">>, <<"v1">>}]}, 2]},
{call, f, none, [1, {[{eq, <<"name">>, <<"v2">>}]}, 2]}]}),
<<"f(1,{name=~`v1|v2`},2)">>),
?assertEqual(format_promql(
{'or',
{union,
[{call, f, none, [1,{[{eq, <<"name">>, <<"v1">>}]}, 2]},
{call, f, none, [1,{[{eq, <<"name">>, <<"v2">>}]}, 3]}]}),
<<"f(1,{name=`v1`},2) or f(1,{name=`v2`},3)">>),
?assertEqual(format_promql(
{'or',
{union,
[{call, f, none, [1,{[{eq, <<"name">>, <<"v1">>}]}, 3]},
{call, f, none, [2,{[{eq, <<"name">>, <<"v2">>}]}, 3]}]}),
<<"f(1,{name=`v1`},3) or f(2,{name=`v2`},3)">>),
?assertEqual(format_promql(
{'or', [{call, f, none, [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>}]}]},
{call, f, none, [{[{eq, <<"name">>, <<"v2">>}]},
{[{eq, <<"name">>, <<"v1">>}]}]}]}),
<<"f({name=`v1`},{name=`v2`}) or f({name=`v2`},{name=`v1`})">>),
?assertEqual(format_promql({[{not_any, <<"name">>,
[<<"v1">>, <<"v2">>, <<"v3">>]}]}),
<<"{name!~`v1|v2|v3`}">>).
{union, [{call, f, none, [{[{eq, <<"name">>, <<"v1">>}]},
{[{eq, <<"name">>, <<"v2">>}]}]},
{call, f, none, [{[{eq, <<"name">>, <<"v2">>}]},
{[{eq, <<"name">>, <<"v1">>}]}]}]}),
<<"f({name=`v1`},{name=`v2`}) or f({name=`v2`},{name=`v1`})">>).

-endif.
18 changes: 9 additions & 9 deletions src/stat_names_mappings.erl
View file
Expand Up @@ -43,7 +43,7 @@ pre_70_stats_to_prom_query("@system-processes" = Section, OrGroupSize, all,
[{[{eq, <<"category">>, <<"system-processes">>}]}] ++
[Ast || M <- SpecialMetrics,
{ok, Ast} <- [pre_70_stat_to_prom_query(Section, M, Opts)]],
[promQL:format_promql({'or', SubGroup})
[promQL:format_promql({union, SubGroup})
|| SubGroup <- misc:split(OrGroupSize, AstList)];
pre_70_stats_to_prom_query("@global", _, all, _Opts) ->
[<<"{category=`audit`}">>];
Expand All @@ -59,7 +59,7 @@ pre_70_stats_to_prom_query(StatSection, OrGroupSize, List, Opts) ->
end
end, [bin(S) || S <- List]),

[promQL:format_promql({'or', SubGroup})
[promQL:format_promql({union, SubGroup})
|| SubGroup <- misc:split(OrGroupSize, AstList)].

pre_70_stat_to_prom_query("@system", <<"rest_requests">>, Opts) ->
Expand Down Expand Up @@ -292,24 +292,24 @@ pre_70_stat_to_prom_query("@eventing", <<"eventing/", Stat/binary>>, _Opts) ->
Metrics = [eventing_metric(bin(M), <<"*">>)
|| M <- eventing_failures()],
{ok, promQL:named(<<"eventing_failed_count">>,
promQL:sum({'or', Metrics}))};
promQL:sum({union, Metrics}))};
[FunctionName, <<"failed_count">>] ->
Metrics = [eventing_metric(bin(M), FunctionName)
|| M <- eventing_failures()],
{ok, promQL:named(<<"eventing_failed_count">>,
promQL:sum_by([<<"functionName">>],
{'or', Metrics}))};
{union, Metrics}))};
[<<"processed_count">>] ->
Metrics = [eventing_metric(bin(M), <<"*">>)
|| M <- eventing_successes()],
{ok, promQL:named(<<"eventing_processed_count">>,
promQL:sum({'or', Metrics}))};
promQL:sum({union, Metrics}))};
[FunctionName, <<"processed_count">>] ->
Metrics = [eventing_metric(bin(M), FunctionName)
|| M <- eventing_successes()],
{ok, promQL:named(<<"eventing_processed_count">>,
promQL:sum_by([<<"functionName">>],
{'or', Metrics}))};
{union, Metrics}))};
[N] ->
{ok, promQL:sum_by([<<"name">>], eventing_metric(N, <<"*">>))};
[FunctionName, N] ->
Expand Down Expand Up @@ -375,8 +375,8 @@ pre_70_stat_to_prom_query(Bucket, <<"couch_docs_data_size">>, _Opts) ->
M = promQL:bucket_metric(<<"kv_ep_db_data_size_bytes">>, Bucket),
{ok, promQL:named(<<"couch_docs_data_size">>, promQL:sum(M))};
pre_70_stat_to_prom_query(Bucket, <<"disk_write_queue">>, _Opts) ->
M = {'or', [promQL:bucket_metric(<<"kv_ep_queue_size">>, Bucket),
promQL:bucket_metric(<<"kv_ep_flusher_todo">>, Bucket)]},
M = {union, [promQL:bucket_metric(<<"kv_ep_queue_size">>, Bucket),
promQL:bucket_metric(<<"kv_ep_flusher_todo">>, Bucket)]},
{ok, promQL:named(<<"kv_disk_write_queue">>, promQL:sum(M))};
pre_70_stat_to_prom_query(Bucket, <<"ep_ops_create">>, Opts) ->
M = promQL:rate(promQL:bucket_metric(<<"kv_vb_ops_create">>, Bucket), Opts),
Expand All @@ -399,7 +399,7 @@ pre_70_stat_to_prom_query(Bucket, <<"ops">>, Opts) ->
<<"del_meta">>, <<"get_meta">>,<<"set_meta">>,
<<"set_ret_meta">>,<<"del_ret_meta">>]}]},
Opts)],
{ok, promQL:named(<<"kv_old_ops">>, promQL:sum({'or', Metrics}))};
{ok, promQL:named(<<"kv_old_ops">>, promQL:sum({union, Metrics}))};
pre_70_stat_to_prom_query(Bucket, <<"vb_total_queue_age">>, _Opts) ->
M = promQL:bucket_metric(<<"kv_vb_queue_age_seconds">>,
list_to_binary(Bucket)),
Expand Down

0 comments on commit 38831e5

Please sign in to comment.