Skip to content

Fix missing topic in dlq producer name when using RetryEnable option#1412

Merged
nodece merged 1 commit intoapache:masterfrom
geniusjoe:bugfix/default-dlq-producer-name
Aug 29, 2025
Merged

Fix missing topic in dlq producer name when using RetryEnable option#1412
nodece merged 1 commit intoapache:masterfrom
geniusjoe:bugfix/default-dlq-producer-name

Conversation

@geniusjoe
Copy link
Contributor

@geniusjoe geniusjoe commented Aug 27, 2025

Fixes #1314
Master Issue: apache/pulsar#21589

Motivation

When creating a consumer with option.RetryEnable option, consumer initialization needs to subscribe both option.Topic and options.DLQ.RetryLetterTopic, it will set option.Topic to empty string and move this topic to option.Topics for subscribing. Codes below:

if options.Topic != "" && len(options.Topics) == 0 {

	if options.RetryEnable {
		... omit code

		if options.Topic != "" && len(options.Topics) == 0 {
			options.Topics = []string{options.Topic, options.DLQ.RetryLetterTopic} # move options.Topic to options.Topics 
			options.Topic = ""
		} else if options.Topic == "" && len(options.Topics) > 0 {
			options.Topics = append(options.Topics, options.DLQ.RetryLetterTopic)  # move options.Topic to options.Topics 
		}
	}

But newDlqRouter() function use option.Topic when generating default producer name, when we set option.RetryEnable to true, default producer name will become to -<subscriptionName>-<consumerName>-<randomString>-DLQ (lose topicName part).

dlq, err := newDlqRouter(client, options.DLQ, options.Topic, options.SubscriptionName, options.Name,

Modifications

Modify pulsar/consumer_impl.go#newDlqRouter() initialization, when enable option.RetryEnable option, we try to find source topic in option.Topics. And if we set option.RetryEnable to false or there are multiple source topics, keep consistent with previous strategy(i.e. empty string).

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as
pulsar/consumer_test.go#TestRLQ
pulsar/consumer_test.go#TestRLQMultiTopics

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@geniusjoe geniusjoe force-pushed the bugfix/default-dlq-producer-name branch from ac04ff6 to 9bd16e5 Compare August 28, 2025 02:58
@nodece nodece merged commit 4392621 into apache:master Aug 29, 2025
7 checks passed
@nodece nodece added this to the v0.17.0 milestone Aug 29, 2025
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