500 errors when deleting connections via management server REST interface

[Matt Bennett]

I've discovered two situations where deleting a connection through the rest interface results in a 500 error.

Situation 1: Delete a connection that existed in a previous vhost.

Most easily explained by the log: https://gist.github.com/mattbennett/4cec602c39b7a738a373

a) connection 127.0.0.1:41142 is accepted
b) vhost is deleted
c) new vhost is created with the same name
d) closing connection 127.0.0.1:41142 generates a 500 error


Situation 2: Delete a connection that has only just been accepted.

Log: https://gist.github.com/mattbennett/4132689345adcb17703c

a) connection 127.0.0.1:59257 is accepted
b) connection 127.0.0.1:59257 is "closed by the management plugin" (via a REST call)

c) boom

I'm less sure this isn't my fault, but a 500 error doesn't seem correct behaviour for rabbit. In case it's interesting, the two connections here are generated by a test that doesn't clean up after itself. We force closed any connection than remain between tests, and sometimes this results in the error above.

Are these possibly two bugs?

[Michael Klishin]

On 18 August 2014 at 21:36:59, Matt Bennett (ma...@bennett.name) wrote:
> > Situation 1: Delete a connection that existed in a previous
> vhost.
>
> Most easily explained by the log: https://gist.github.com/mattbennett/4cec602c39b7a738a373
>
> a) connection 127.0.0.1:41142 is accepted
> b) vhost is deleted
> c) new vhost is created with the same name
> d) closing connection 127.0.0.1:41142 generates a 500 error

What version do you use? I'm not sure this is a common use case. Perhaps
forcefully closing all connections when vhost is deleted is the right thing
to do.

> Situation 2: Delete a connection that has only just been accepted.
>
> Log: https://gist.github.com/mattbennett/4132689345adcb17703c
>
> a) connection 127.0.0.1:59257 is accepted
> b) connection 127.0.0.1:59257 is "closed by the management
> plugin" (via a REST call)
> c) boom
>
> I'm less sure this isn't my fault, but a 500 error doesn't seem
> correct behaviour for rabbit. In case it's interesting, the
> two connections here are generated by a test that doesn't clean
> up after itself. We force closed any connection than remain between
> tests, and sometimes this results in the error above.

This is a race condition: it takes some time (however small) to set up everything
related to a connection, so  I think returning 404 is the best solution
we can offer.
--
MK

Staff Software Engineer, Pivotal/RabbitMQ

[Matt Bennett]

On Monday, August 18, 2014 7:02:13 PM UTC+1, Michael Klishin wrote:

On 18 August 2014 at 21:36:59, Matt Bennett (ma...@bennett.name) wrote:
> > Situation 1: Delete a connection that existed in a previous  
> vhost.
>  
> Most easily explained by the log: https://gist.github.com/mattbennett/4cec602c39b7a738a373  
>  
> a) connection 127.0.0.1:41142 is accepted
> b) vhost is deleted
> c) new vhost is created with the same name
> d) closing connection 127.0.0.1:41142 generates a 500 error  

What version do you use? I'm not sure this is a common use case. Perhaps
forcefully closing all connections when vhost is deleted is the right thing
to do.

Ah sorry, I forgot to mention versions. This was on RabbitMQ 3.3.5, Erlang R14B04 and Ubuntu 12.04.

Indeed, during the course of this investigation I did think "why don't we just close the connections first", and sure enough that does avoid this situation. But then our test suite started failing with the error below.

 

> Situation 2: Delete a connection that has only just been accepted.  
>  
> Log: https://gist.github.com/mattbennett/4132689345adcb17703c  
>  
> a) connection 127.0.0.1:59257 is accepted
> b) connection 127.0.0.1:59257 is "closed by the management  
> plugin" (via a REST call)
> c) boom
>  
> I'm less sure this isn't my fault, but a 500 error doesn't seem  
> correct behaviour for rabbit. In case it's interesting, the  
> two connections here are generated by a test that doesn't clean  
> up after itself. We force closed any connection than remain between  
> tests, and sometimes this results in the error above.

This is a race condition: it takes some time (however small) to set up everything
related to a connection, so  I think returning 404 is the best solution
we can offer.

A 404 would be perfect! In fact we already assume there'll be a race when closing connections and explicitly silence 404s.

[Michael Klishin]

On 18 August 2014 at 22:24:45, Matt Bennett (ma...@bennett.name) wrote:
> > Indeed, during the course of this investigation I did think
> "why don't we just close the connections first", and sure enough
> that does avoid this situation. But then our test suite started
> failing with the error below.

…

> A 404 would be perfect! In fact we already assume there'll be a
> race when closing connections and explicitly silence 404s.

Thanks Matt, I'll file bugs for both issues.
--