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

Bugfix: Use federated queue's name if upstream queue name is empty #562

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

spuun
Copy link
Member

@spuun spuun commented Aug 15, 2023

WHAT is this pull request doing?

When an federation upstream is created without queue name the queue name on the upstream server was auto generated (amq.gen-*) because empty string was passed in the declare call.

This will extend the nil check of queue name to also check for empty string.

HOW can this pull request be tested?

  1. Create an federation upstream without any exchange or queue parameters
  2. Create a queue on downstream
  3. Apply federation to the queue
  4. Verify that a queue has been declared on upstream with the same name as the federated queue

When federation parameters are stored queue is stored as an empty string
if no value is given. This caused the upstream queue name to be declared
with empty string as name, which means that the queue name will be auto
generated.

This change will treat both nil and empty string as "no value" and
fallback to use the federated queues' name.
@github-actions
Copy link

Main benchmark
'Average publish rate: 349534.8 msgs/s'
'Average consume rate: 348321.2 msgs/s'

PR benchmark
'Average publish rate: 354001.6 msgs/s'
'Average consume rate: 351782.1 msgs/s'

Keep in mind, these numbers are not representative of LavinMQ's peak performance.
It is rather an indication of how the changes of this pull request affects the performance of the main branch.

@carlhoerberg carlhoerberg merged commit 178373f into main Aug 22, 2023
@carlhoerberg carlhoerberg deleted the federation-bug-upstream-queue branch August 22, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants