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

A new local random exchange #8334

Closed
wants to merge 11 commits into from
Closed

Conversation

MarcialRosales
Copy link
Contributor

@MarcialRosales MarcialRosales commented May 25, 2023

Proposed Changes

This is a new exchange type which works like the random exchange but only selects from bound queues that are on the same local node as the publishing connection.

In scenarios where at least one queue per RabbitMQ node is bound this exchange provides high publishing availability with no additional network hops which also reduces publishing latency.

Use case scenario

In a RabbitMQ cluster of three nodes: A, B and C.

3 classic (v2) queues are declared, one on each node: q-A, q-B and q-C and they are all bound to the same instance of the rabbitmq_local_exchange type.

A message published to the exchange on node A will only be delivered to queue: a-A, a message published on node B will only be delivered to q-B and so on. This is enough to provide very high publishing availability at very low latency.

Consumers

The consuming side is somewhat more complex. Users need to ensure that there are consuming applications to cover all queues and that at least one consumer is connected to each node. This means there need to be at least one consuming connection per RabbitMQ node. In this instance we need at least 3 consuming applications: c-A, c-B and c-C each connected to their own RabbitMQ node and consuming from the queue declared on each node: c-A is connected to node A and consumes from q-A etc.

To provide more consumption availability each consumer could connect to all nodes and consume from all queues. If competing consumption isn't desired then the queues could use the single active consumer feature.

This pattern does push quite a bit of topological knowledge onto the consuming applications, for example they need to know which queues are running on which nodes so that they can ensure we don't accidentally introduce and additional network hop for consumer delivery. But once this is set up very low latency consumption at high availability is possible.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • [X ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)
  • Build system and/or CI

TODO

  • Move to core.
  • Rename. to rabbitmq_local_random_exchange / x-local-random
  • Disallow non classic queues to be bound in the validate_binding/2 callback.
  • Only select a random local queue, do not fall back to selecting a random remote queue (as the code currently does)
  • Reformat towards modern erlang style (4 space indents. no redundant whitespace / indentation).

@kjnilsson kjnilsson changed the title Prototype local exchange A new local exchange type May 25, 2023
@MarcialRosales MarcialRosales force-pushed the prototype-local-exchange branch 4 times, most recently from b33aa2a to cd5de9b Compare May 29, 2023 06:31
@kjnilsson
Copy link
Contributor

I am wondering about the current behaviour with this exchange. Currently if there are more than one locally bound queue it will select a random one and if there are no locally bound queues it will select a random one from the full set of queues. So in the latter case it is no longer "local".

The alternative behaviour would be that it will deliver to all locally bound queues and never to queues that are not locally bound.

Looking for thoughts on this.

@dumbbell
Copy link
Member

dumbbell commented Jun 7, 2023

If there are no locally bound queues, I would expect the message to be treated like an unroutable message. My reasoning is that avoiding inter-node traffic contributes to maintaining the low latency sold by this exchange. And there is "local" in the name, not "maybe_local". Or, there could be an argument to configure the behavior.

If there are many locally bound queues, I'm not sure. Perhaps the routing queue should be taken into account?

Unrelated to @kjnilsson questions, is there any value in providing this exchange as a plugin, as opposed to inside the broker?

@kjnilsson
Copy link
Contributor

@dumbbell we had a chat in slack about this and it seems the consensus is it should be strictly local and if a fallback is needed then an alternative exchange should be configured.

@kjnilsson
Copy link
Contributor

Unrelated to @kjnilsson questions, is there any value in providing this exchange as a plugin, as opposed to inside the broker?

Probably not but all non-standard exchange types are AFAIK plugins so it is just following suite.

@dumbbell
Copy link
Member

dumbbell commented Jun 7, 2023

@dumbbell we had a chat in slack about this and it seems the consensus is it should be strictly local and if a fallback is needed then an alternative exchange should be configured.

Ah ok, I didn't follow the discussion there, but apparently we reached the same conclusion :-) I updated my message above to give my reasoning about this.

Unrelated to @kjnilsson questions, is there any value in providing this exchange as a plugin, as opposed to inside the broker?

Probably not but all non-standard exchange types are AFAIK plugins so it is just following suite.

That's what I thought but maybe it's a good time to revisit this decision. I see no value in the plugin format here. However, I see plenty of issues:

  • It's a burden for developers because we have to deal with the code split into multiple Erlang applications (painful when there is an API change affecting many modules).
  • It's a burden for the build system too and we maintain two of them.
  • And above all, it's a usability problem for users: they can't just declare the exchange and enjoy it, they need to enable a plugin first (or worse, they need to contact their local admin to do it for them).

We are not saving any resources by putting that in a plugin.

@kjnilsson
Copy link
Contributor

maybe it's a good time to revisit this decision.

I'm up for this

filter_local_queue(Q) ->
{ok, Queue} = rabbit_amqqueue:lookup(Q),
Qpid = amqqueue:get_pid(Queue),
erlang:node(Qpid) =:= node().
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw a badarg for quorum queues, as it is not an Erlang pid.

@lhoguin
Copy link
Contributor

lhoguin commented Jun 26, 2023

Should we make this NOT a plugin then, and if so where should be the module? In rabbit directly? Happy to do the change after there's some consensus.

@kjnilsson kjnilsson changed the title A new local exchange type Local random exchange Jun 29, 2023
@kjnilsson kjnilsson changed the title Local random exchange A new local random exchange Jun 29, 2023
@kjnilsson
Copy link
Contributor

I think the TODO list of this PR is:

  1. Move to core.
  2. Rename. to rabbitmq_local_random_exchange / x-local-random
  3. Disallow non classic queues to be bound in the validate_binding/2 callback.
  4. Only select a random local queue, do not fall back to selecting a random remote queue (as the code currently does)
  5. Reformat towards modern erlang style (4 space indents. no redundant whitespace / indentation).

@kjnilsson
Copy link
Contributor

Replaced by #10091

@kjnilsson kjnilsson closed this Dec 11, 2023
@mkuratczyk mkuratczyk deleted the prototype-local-exchange branch December 27, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants