Topology Recovery queue binding fails

[spatula75]

Detailed discussion at: https://groups.google.com/forum/#!topic/rabbitmq-users/c76nrrSWgs8

It appears that in some cases, during queue binding recovery, the channel used is closed, possibly because channel.getDelegate().queueBind(...) is called instead of channel.queueBind(...).

[spatula75]

My initial hypothesis looks wrong; even calling channel.queueBind() without .getDelegate still throws an exception that the channel is closed due to clean shutdown. Will try to investigate further. Fortunately the steps to reproduce are pretty simple when one can yank an ethernet cable.

[spatula75]

I think the problem might be that it isn't always the case that the channel a queue was originally bound with is still around and open later when it comes time to recover the queue binding, though it isn't clear to me yet why it is that the queue recovery works but the queue binding recovery doesn't work (in our code these two are both done using the same channel) unless the queue hasn't yet been auto-deleted at the time recovery is attempted, though I'd still expect to get an exception complaining that the channel is closed in that case.

I did note just now in digging further that we don't see this happen in the event that we've never restarted a consumer-- meaning we've never closed the Consumer's channel, which is the same channel we used to do our queueDeclare and queueBind. If, however, we have previously restarted a consumer (by canceling it, closing its channel, then doing a fresh queueDeclare, queueBind, and basicConsume), then we yank the ethernet and plug it back in, we do get the exception.

However I would expect that since we did a new queueDeclare and queueBind on the new channel we created for the new consumer, then this channel should have been recovered for the RecordedQueueBinding, in the same way it was for the RecordedQueue. So I think the mystery to solve is how the behavior of channel.queueDeclare differs from channel.queueBind with respect to recording.

[spatula75]

I found the problem, and depending on how you look at it, it's either a need of documentation, a need of my having my head examined, or a small code change. In AutorecoveringConnection#recordQueueBinding:

        if (!this.recordedBindings.contains(binding)) {
            this.recordedBindings.add(binding);
        }

So if you add a new binding, which is exactly the same as the existing binding, it doesn't get replaced with the new binding that contains the new channel (because the RecordedBinding .equals() method doesn't include the Channel when determining equality, and it probably shouldn't).

By contrast, recordQueue just does a recordedQueues.put, probably because it's easy to do that as it's a Map, not a List.

Workaround: delete the binding before creating the binding with the same values but a new channel.

Possible fix, but I'll need your considered opinion on this one, @michaelklishin, is changing it to

if (recordedBindings.contains(binding)) { 
    recordedBindings.remove(binding)
} 
recordedBindings.add(binding);

[michaelklishin]

@spatula75 good find. Recorded bindings caching channels is indeed tricky. I think removing a binding that already exists is reasonable.

[michaelklishin]

@spatula75 thank you for digging this one out and contributing a fix!