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

Conversation

michaelklishin
Copy link
Member

We group x-death header values before processing them
to make sure there's only one per {queue, reason}.

Fixes #152.

We group x-death header values before processing them
to make sure there's only one per {queue, reason}.
@michaelklishin michaelklishin added this to the n/a milestone May 9, 2015
@michaelklishin
Copy link
Member Author

Test case: rabbitmq/rabbitmq-java-client#56.

[X] ->
maybe_append_to_event_group(
ensure_xdeath_event_count(X), Q, R, Acc);
[X|_] = Xs when is_list(Xs) ->
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 to test for is_list here when you are matching already for a list?

@michaelklishin
Copy link
Member Author

@videlalvaro ready for another round. I'll re-format with Emacs once we agree on everything else.

@videlalvaro
Copy link
Contributor

About the sort, I think lists:partition should take care of partitioning the list by duplicates, provided the list is sorted

@videlalvaro
Copy link
Contributor

Also see the comment related to duplicated fun code in queue_and_reason_matcher

@michaelklishin
Copy link
Member Author

Sorting is not sufficient: we may have duplicates in the input that foldl would still iterate over, so they need to be checked. We can avoid an O(n) check by keeping a set of {queue, reason} pairs in the accumulator.

[{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.

@videlalvaro
Copy link
Contributor

Added some comments, after those, I think just the indentation ones remain

videlalvaro added a commit that referenced this pull request May 11, 2015
Support x-death event values from before #78
@videlalvaro videlalvaro merged commit ac896b7 into stable May 11, 2015
@dumbbell dumbbell deleted the rabbitmq-server-152 branch January 2, 2018 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants