-
Notifications
You must be signed in to change notification settings - Fork 370
fix: add DLQPolicy.DeadLetterTopicProducerName #1417
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
fix: add DLQPolicy.DeadLetterTopicProducerName #1417
Conversation
… instead of the RLQ producer name
lhotari
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where the DLQ (Dead Letter Queue) producer incorrectly uses the RLQ (Retry Letter Queue) producer name when sending messages to the DLQ topic. The fix adds a new DeadLetterTopicProducerName field to the DLQPolicy struct to allow specifying a custom producer name for the DLQ or letting the system generate one using the existing naming pattern.
- Adds
DeadLetterTopicProducerNamefield toDLQPolicystruct for configuring DLQ producer names - Updates DLQ router logic to use the new field instead of incorrectly using RLQ producer name
- Adds comprehensive integration tests to verify both custom and auto-generated DLQ producer names
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pulsar/consumer.go | Adds the new DeadLetterTopicProducerName field to the DLQPolicy struct with documentation |
| pulsar/dlq_router.go | Updates producer name logic to check and use the new DeadLetterTopicProducerName field |
| pulsar/consumer_test.go | Adds two integration tests to verify the new functionality works correctly |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Looks like CI failed with a timeout in the |
Motivation
When the retry mechanism is enabled, the RLQ producer first sends messages to the RLQ topic using the producer name if passed in the producer options inside the
DLQPolicy.However, with
DLQPolicy.ProducerOptions.Nameset, when the delivery count is reached and the consumerNack()the messages, the DLQ producer sending messages to the DLQ uses RLQ producer name.This is an issue, especially for message deduplication process where each producers must have a globally unique name for a given topic.
Modifications
This PR adds the
DeadLetterTopicProducerNamefield to theDLQPolicyin order to either allow using a custom producer name or let the client create the producer name with the existing naming pattern.Verifying this change
TestWithoutDeadLetterTopicDeadLetterTopicProducerNameTestWithDeadLetterTopicDeadLetterTopicProducerNameDoes this pull request potentially affect one of the following parts:
Documentation
DeadLetterTopicProducerNamefield added is documented where it's defined in theDLQPolicystruct