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

pubsub/rabbit: pass requeue: false into amqpChannel.Nack #2952

Closed
kmlebedev opened this issue Feb 2, 2021 · 10 comments
Closed

pubsub/rabbit: pass requeue: false into amqpChannel.Nack #2952

kmlebedev opened this issue Feb 2, 2021 · 10 comments

Comments

@kmlebedev
Copy link

kmlebedev commented Feb 2, 2021

I cannot configure RabbitMQ delay retry/schedule with Dead Letter Exchange without pass requeue: false to reject in DLX
https://medium.com/@kiennguyen88/rabbitmq-delay-retry-schedule-with-dead-letter-exchange-31fb25a440fc

return ch.ch.Nack(tag, false, true) // multiple=false: acking only this ID, requeue: true to redeliver

And gets flag redelivered true

@vangent
Copy link
Contributor

vangent commented Feb 2, 2021

If you don't want the message to be redelivered, just Ack it instead of Nack.

That is what the sample code in the article you linked to do does -- it acks the message from WorkQueue, and publishes it to RetryQueue on failure.

@vangent vangent closed this as completed Feb 2, 2021
@kmlebedev
Copy link
Author

kmlebedev commented Feb 2, 2021

If you don't want the message to be redelivered, just Ack it instead of Nack.

That is what the sample code in the article you linked to do does -- it acks the message from WorkQueue, and publishes it to RetryQueue on failure.

that's exactly what I'm doing here https://github.com/chrislusf/seaweedfs/blob/master/weed/replication/sub/notification_gocdk_pub_sub.go#L51

But by default Nack requeue is true, then messages not delivered to DLX

When requeue is true, request the server to deliver this message to a different
consumer.  If it is not possible or requeue is false, the message will be
dropped or delivered to a server configured dead-letter queue.

https://github.com/streadway/amqp/blob/e6b33f460591b0acb2f13b04ef9cf493720ffe17/delivery.go#L157

I want on the first failure try
send Nack with requeue is true
On the second try, then if Redelivered is true
https://github.com/streadway/amqp/blob/e6b33f460591b0acb2f13b04ef9cf493720ffe17/delivery.go#L54
send Nack with requeue is false , then messages delivered to DLX , then TTL 3 min messages delivered to WorkQueue

@kmlebedev
Copy link
Author

kmlebedev commented Feb 2, 2021

@vangent
Copy link
Contributor

vangent commented Feb 2, 2021

I'm still not sure I understand exactly what you are trying to do, but if you want different behavior from Nack depending on the context, that's not going to be possible through the Go CDK pubsub API.

You can always use the As escape hatches to make more specific calls to the underlying service if that's what you need.

If you think there's a way to make this work using an Option or something, then please reopen and send a PR.

@kmlebedev
Copy link
Author

If you think there's a way to make this work using an Option or something, then please reopen and send a PR.

At least in the interface, you can implement the function message.Reject()
https://github.com/streadway/amqp/blob/e6b33f460591b0acb2f13b04ef9cf493720ffe17/delivery.go#L143

@vangent
Copy link
Contributor

vangent commented Feb 2, 2021

The pubsub interface for Go CDK is used for multiple providers, not just Rabbit. I don't think Reject makes sense across all of those providers (it's basically Nack).

And sorry, I shouldn't have called it an interface, as it's a concrete type:
https://github.com/google/go-cloud/blob/master/pubsub/pubsub.go

@kmlebedev
Copy link
Author

kmlebedev commented Feb 3, 2021

The pubsub interface for Go CDK is used for multiple providers, not just Rabbit. I don't think Reject makes sense across all of those providers (it's basically Nack).

And sorry, I shouldn't have called it an interface, as it's a concrete type:
https://github.com/google/go-cloud/blob/master/pubsub/pubsub.go

And how, for different types, is the re-placing of a message in the queue through DLX.RetryExchange?

In a specific case for type RabbitMQ, your interface is not working, where the FIFO principle is used.
for example we have 100 messages , where 10 is failure message we send Nack , then deliver this message to same consumer and we get an endless loop. The remaining 90 messages will not be processed

It turns out more harm than good from Naсk with requeue is true

@vangent
Copy link
Contributor

vangent commented Feb 3, 2021

The semantics for Nack are documented, see here:
https://pkg.go.dev/gocloud.dev/pubsub#Message.Nack

Nack is a performance optimization for retrying transient failures. It must not be used for message parse errors or other messages that the application will never be able to process: calling Nack will cause them to be redelivered and overload the server. Instead, an application should call Ack and log the failure in some monitored way.

It sounds like RabbitMQ has some mechanism for automatically adding nacked messages to a separate queue that Go CDK does not support because of the way we call Nack. Your options are either:
a) Use As to access the RabbitMQ-specific functionality (https://pkg.go.dev/gocloud.dev/pubsub#Subscription.As).
b) Enqueue the failed messages in the separate queue in your code instead of configuring RabbitMQ to do it.
c) If you think there's a way to make this work via code changes in Go CDK, please suggest something (as a PR or a description of what that PR would look like).

HTH

@kmlebedev
Copy link
Author

The semantics for Nack are documented, see here:
https://pkg.go.dev/gocloud.dev/pubsub#Message.Nack

Nack is a performance optimization for retrying transient failures. It must not be used for message parse errors or other messages that the application will never be able to process: calling Nack will cause them to be redelivered and overload the server. Instead, an application should call Ack and log the failure in some monitored way.

It sounds like RabbitMQ has some mechanism for automatically adding nacked messages to a separate queue that Go CDK does not support because of the way we call Nack. Your options are either:
a) Use As to access the RabbitMQ-specific functionality (https://pkg.go.dev/gocloud.dev/pubsub#Subscription.As).
b) Enqueue the failed messages in the separate queue in your code instead of configuring RabbitMQ to do it.
c) If you think there's a way to make this work via code changes in Go CDK, please suggest something (as a PR or a description of what that PR would look like).

HTH

Yes I just use a) thanks
But option b) loss of message is allowed in certain cases
for option с) I'm not that good at knowing other types of queues

@vangent
Copy link
Contributor

vangent commented Feb 3, 2021

Sounds good. To be clear though, c) does not require touching anything other than the Rabbit driver.

For example, it would be possible to make requeue=false all the time (https://github.com/google/go-cloud/blob/master/pubsub/rabbitpubsub/amqp.go#L110) by adding a SubscriptionOption (https://github.com/google/go-cloud/blob/master/pubsub/rabbitpubsub/rabbit.go#L138) called something like RequeueNacks and plumbing it through.

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

No branches or pull requests

2 participants