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

[events] Support ephemeral consumers #184

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

rizzza
Copy link
Contributor

@rizzza rizzza commented Jan 19, 2024

loadbalancer-manager-haproxy does not have a need for durable message queues, as we want each instance to consume all messages received related to loadbalancers.

Currently, we generate a unique durable queue for each manager instance, which eventually puts strain on nats with the number of consumers that can be generated over time. This PR aims to put support for ephemeral consumers back into event subscriptions, if requested, by leaving QueueGroup empty in the nats config.

Signed-off-by: Matt Siwiec <rizzza@users.noreply.github.com>
@rizzza rizzza requested a review from a team as a code owner January 19, 2024 20:41
nicolerenee
nicolerenee previously approved these changes Jan 19, 2024
Copy link
Member

@nicolerenee nicolerenee left a comment

Choose a reason for hiding this comment

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

It's been a minute since I touched the NATS bits but looking through this and the NATS godocs this looks good to me. I'll leave it unmerged for now so anyone else from sig-events can review.

Copy link
Contributor

@mikemrm mikemrm left a comment

Choose a reason for hiding this comment

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

Other than this small change, this looks good.

@@ -55,7 +55,11 @@ type NATSConfig struct {

// Configured checks whether the provider has been configured.
func (c NATSConfig) Configured() bool {
return c.URL != "" || c.QueueGroup != ""
if c.QueueGroup == "" {
c.logger.Warn("NATS QueueGroup is not set. Subscriptions will not be durable.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This warning should go in NewNATSConnection function. This method is to determine whether this config has a configuration defined.

Signed-off-by: Matt Siwiec <rizzza@users.noreply.github.com>
@rizzza rizzza added this pull request to the merge queue Jan 22, 2024
Merged via the queue into infratographer:main with commit a5142b6 Jan 22, 2024
6 checks passed
@rizzza rizzza deleted the support-ephemeral-consumers branch January 22, 2024 20:24
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.

3 participants