Replace TCP acceptor pool with Ranch

[essen]

We are currently investigating how feasible it would be to just use Ranch for handling connections. Depending on the results of this investigation, the custom acceptor pool would be dropped and Ranch used directly. Our key goal is to not have to maintain our own library, but we have to stay compatible w.r.t. network-related configuration.

[essen]

The acceptor pools are quite similar.

Configuration is basically the same (Ranch is filtering out a few options, like SSL's depth option or TCP's exit_on_close option, but adding those to Ranch is trivial).

Concepts of listener and acceptors are very similar, it should be fairly straightforward to replace them.

The supervision tree for connections in RabbitMQ is a bit complex but I don't think it would be a problem. The ranch_conns_sup supervisor would become rabbit_tcp_client_sup (registered name) handled by rabbit_client_sup module. The difference would be mainly that ranch_conns_sup is a custom supervisor, while rabbit_client_sup is supervisor2. The supervision tree would otherwise stay unchanged.

I estimate this would take about two days to get something up and running, and probably some time after that to resolve unforeseen issues.

[michaelklishin]

Why does Ranch filter out TLS depth? It is for sure used by some of our users.

[essen]

Rather a correct wording would be: Ranch validates options (and sets some required defaults at the same time) and the list of allowed options was done on a use case basis.

So if depth isn't there yet, it's that no Ranch user has ever used it. :-)

But it's a two lines patch to add options so it won't take me long.

[michaelklishin]

@essen excellent, then lets make sure we extend Ranch to accept all ssl_options known and go for it. Keep in mind that the pool is used by plugins such as STOMP and MQTT. They don't do anything sophisticated, just provide more data points for testing.

[essen]

Alright.

[essen]

OK putting some notes here mostly so I don't forget things:

• Ranch is missing some TCP and SSL options
• Ranch does not support callbacks for oninit and onterminate; we do not need to add those, we can just keep the tcp_listener process and let it do the callback handling like it does today
• When starting the listener, use ranch:child_spec instead of what we currently do; put the Ranch child spec side by side with tcp_listener, and start them in that order; put them under tcp_listener_sup
• Make sure rabbit_tcp_client_sup starts before the listeners (this is already the case, just a remainder); we need it to stop after the listeners on shutdown (see below)
• The reader process will be supervised by the ranch part of the supervision tree (but still under the rabbit application)
• The connection related processes (queue, apparently 2 hearbeat processes and the channel_sup_sup) can still sit under rabbit_connection_sup; however rabbit_connection_helper_sup is not needed anymore
• Link the reader process and the rabbit_connection_sup process so that they both die together (basically simulating what having them under the rabbit_connection_sup supervisor would do)

It's interesting that the process that is supervised and the process that receives ownership of the socket are not the same. I will open a ticket about that and think about it for Ranch 2. We can then later on have everything in a proper supervision tree (not needing rabbit_tcp_client_sup anymore either). In the meantime, we need to use the link trick to have the same behavior as we had previously.

Doing the integration first, options can be added to Ranch when everything else starts working.

[essen]

ninenines/ranch#117, ninenines/ranch#121 and ninenines/ranch#122 are required changes for this ticket to be resolved, though neither will prevent me from finishing the proof of concept.

[essen]

The branch: https://github.com/rabbitmq/rabbitmq-server/tree/rabbitmq-server-260

The current WIP commit: ad9753a

[essen]

• Check that we still do what Avoid spamming error_logger ninenines/ranch#122 talks about, regardless of that ticket being closed.

No changes needed in Ranch, but need to make sure we still do this in RabbitMQ itself.

[essen]

• Move the rest of the connection-related processes under the ranch listener's supervision tree once Add a password verification command to rabbitmqctl #119 is done.

[michaelklishin]

@essen are you sure you mean #122? It seems unrelated to Ranch (or client connections).

[essen]

Damn I meant Ranch's 122, sorry for the spam. :-)

[essen]

Branch has been updated and works against current Ranch master but there's still work to do. :-)

[essen]

Most recent commit is 89a51ca.

A few more things left to do, tests to be tested and then integrating that in the build properly.

[essen]

Pushed another update in 23421ec. I'm at the point where I did everything I could do on my own and I'll need help to integrate in the build system to finish. There's also an open ended question in the diff that will involve questions or investigation (search for "todo" if curious).

[michaelklishin]

@essen thank you! @dumbbell should we put this on hold until you finish erlang.mk work?

[essen]

For future reference: I will also need to update the AMQP 1.0 plugin when the time comes (stumbled upon a compile issue while doing other things).

[michaelklishin]

I did some fairly extensive testing and none of the minor regressions I could find were Ranch related. Thank you, @essen!

[essen]

Woo!!