Fix max retry count for new version of RabbitMQ x-death format

[jaronkk]

As of March 26, RabbitMQ consolidated the x-death header to have only one entry per queue. A new "count" value was added to track the number of times it's been in a queue.

I've adjusted the failure_count method so it should work for both the old and new formats for the x-death header.

rabbitmq/rabbitmq-server#78
rabbitmq/rabbitmq-server#79

[justinmills]

If I understand, for the "retired once case", using an older version of rabbit, there will be no count attribute, and the above x_death_array count will be 1, so x_death_array.first['count'] will return nil, and so you'll return 1, does that sound about right?

Also, what happens when x_death_array is 0? Won't this raise an exception? Should there be some protection if that's the case (although it might not be something Rabbit would intentionally do).

[jaronkk]

Your interpretation for the "retired once case" is correct. It might be a little more clear if the logic was refactored to something like this:

if x_death_array.count == 1 && x_death_array.first['count']
  x_death_array.first['count']
else
  x_death_array.count
end

For your second question, the x-death headers are always an array. If there isn't an entry, then it will return x_death_array.count which will be 0, which is the same thing that happens in the current version of the code. This method is returning the failure count, which will always be 0 the first time it's in the queue.

[justinmills]

Yep, gotcha. Sorry I missed that earlier.

[justinmills]

Thanks for updating this for newer versions of RabbitMQ! I'm not sure the extra protection is a requirement based on how RabbitMQ works and whether or not the x-death headers are ever not there, but might be worthwhile.

[michaelklishin]

It's not one per queue but one per {queue, reason}.

[jaronkk]

@justin-yesware The x-death headers aren't there the first time an entry is in the queue, in which case the failure_count method will return 0 (handled by the first if statement in failure_count).

@michaelklishin That makes sense.

Would it make sense to change the logic to do a check for a count key, and then sum the counts?

[jaronkk]

@justin-yesware I updated the logic here so it's more clear when it's using the new vs legacy code. I also updated it to do a sum of the counts in case there were failures for multiple reasons.

[justinmills]

Changes look good and the internal sum should address the issues @michaelklishin brought up.

Also, this does read a lot cleaner, thanks.

[jondot]

Awesome insights guys, thanks @jaronkk!