Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support x-death event values from before #78 #153

Merged
merged 6 commits into from
May 11, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 74 additions & 15 deletions src/rabbit_dead_letter.erl
Original file line number Diff line number Diff line change
Expand Up @@ -84,36 +84,95 @@ x_death_event_key(Info, Key, KeyType) ->
{value, {Key, KeyType, Val}} -> Val
end.

maybe_append_to_event_group(Table, _Key, _SeenKeys, []) ->
[Table];
maybe_append_to_event_group(Table, {_Queue, _Reason} = Key, SeenKeys, Acc) ->
case sets:is_element(Key, SeenKeys) of
true -> Acc;
false -> [Table | Acc]
end.

group_by_queue_and_reason([]) ->
[];
group_by_queue_and_reason([Table]) ->
[Table];
group_by_queue_and_reason(Tables) ->
{_, Grouped} =
lists:foldl(
fun ({table, Info}, {SeenKeys, Acc}) ->
Q = x_death_event_key(Info, <<"queue">>, longstr),
R = x_death_event_key(Info, <<"reason">>, longstr),
Matcher = queue_and_reason_matcher(Q, R),
{Matches, _} = lists:partition(Matcher, Tables),
{Augmented, N} = case Matches of
[X] -> {X, 1};
[X|_] = Xs -> {X, length(Xs)}
end,
Key = {Q, R},
Acc1 = maybe_append_to_event_group(
ensure_xdeath_event_count(Augmented, N),
Key, SeenKeys, Acc),
{sets:add_element(Key, SeenKeys), Acc1}
end, {sets:new(), []}, Tables),
Grouped.

update_x_death_header(Info, Headers) ->
Q = x_death_event_key(Info, <<"queue">>, longstr),
R = x_death_event_key(Info, <<"reason">>, longstr),
case rabbit_basic:header(<<"x-death">>, Headers) of
undefined ->
rabbit_basic:prepend_table_header(<<"x-death">>,
rabbit_basic:prepend_table_header(
<<"x-death">>,
[{<<"count">>, long, 1} | Info], Headers);
{<<"x-death">>, array, Tables} ->
%% group existing x-death headers in case we have some from
%% before rabbitmq-server#78
GroupedTables = group_by_queue_and_reason(Tables),
{Matches, Others} = lists:partition(
fun ({table, Info0}) ->
x_death_event_key(Info0, <<"queue">>, longstr) =:= Q
andalso x_death_event_key(Info0, <<"reason">>, longstr) =:= R
end, Tables),
queue_and_reason_matcher(Q, R),
GroupedTables),
Info1 = case Matches of
[] ->
[{<<"count">>, long, 1} | Info];
[{table, M}] ->
case x_death_event_key(M, <<"count">>, long) of
undefined ->
[{<<"count">>, long, 1} | M];
N ->
lists:keyreplace(
<<"count">>, 1, M,
{<<"count">>, long, N + 1})
end
increment_xdeath_event_count(M)
end,
rabbit_misc:set_table_value(Headers, <<"x-death">>, array,
rabbit_misc:set_table_value(
Headers, <<"x-death">>, array,
[{table, rabbit_misc:sort_field_table(Info1)} | Others])
end.

ensure_xdeath_event_count({table, Info}, InitialVal) when InitialVal >= 1 ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there cases where InitialVal might be less than one? If yes, the caller will get an undef error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. If we have an event, the number of times it happened is >= 1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then what's the point of having those InitialVal guards?
On Mon, May 11, 2015 at 12:03 PM Michael Klishin notifications@github.com
wrote:

In src/rabbit_dead_letter.erl
#153 (comment)
:

                 end,
         rabbit_misc:set_table_value(Headers, <<"x-death">>, array,
  •          [{table, rabbit_misc:sort_field_table(Info1)} | Others])
    
  •            [{table, rabbit_misc:sort_field_table(Info1)} | Others])
    
  • end.

+ensure_xdeath_event_count({table, Info}, InitialVal) when InitialVal >= 1 ->

No. If we have an event, the number of times it happened is >= 1.


Reply to this email directly or view it on GitHub
https://github.com/rabbitmq/rabbitmq-server/pull/153/files#r30026279.

{table, ensure_xdeath_event_count(Info, InitialVal)};
ensure_xdeath_event_count(Info, InitialVal) when InitialVal >= 1 ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the guard here? In this line https://github.com/rabbitmq/rabbitmq-server/pull/153/files#diff-e2043d9f095f7dc4abc052dd9fae7599R109 the list passed there must have at least one element for the case to match.

case x_death_event_key(Info, <<"count">>, long) of
undefined ->
[{<<"count">>, long, InitialVal} | Info];
_ ->
Info
end.

increment_xdeath_event_count(Info) ->
case x_death_event_key(Info, <<"count">>, long) of
undefined ->
[{<<"count">>, long, 1} | Info];
N ->
lists:keyreplace(
<<"count">>, 1, Info,
{<<"count">>, long, N + 1})
end.

queue_and_reason_matcher(Q, R) ->
F = fun(Info) ->
x_death_event_key(Info, <<"queue">>, longstr) =:= Q
andalso x_death_event_key(Info, <<"reason">>, longstr) =:= R
end,
fun({table, Info}) ->
F(Info);
(Info) when is_list(Info) ->
F(Info)
end.

per_msg_ttl_header(#'P_basic'{expiration = undefined}) ->
[];
per_msg_ttl_header(#'P_basic'{expiration = Expiration}) ->
Expand Down Expand Up @@ -178,7 +237,7 @@ log_cycle_once(Queues) ->
true -> ok;
undefined -> rabbit_log:warning(
"Message dropped. Dead-letter queues cycle detected" ++
": ~p~nThis cycle will NOT be reported again.~n",
": ~p~nThis cycle will NOT be reported again.~n",
[Queues]),
put(Key, true)
end.