pulsar-client-go icon indicating copy to clipboard operation
pulsar-client-go copied to clipboard

[Issue 664] fix ReconsumeLater panic

Open crossoverJie opened this issue 2 years ago • 7 comments

Fixes #664

Motivation

Removing possible panic.

Modifications

Add nil check and more friendly tips.

Verifying this change

  • [x] Make sure that the change passes the CI checks.

crossoverJie avatar Mar 25 '22 17:03 crossoverJie

@Shoothzj would you mind enabling the workflows and reviewing them?

crossoverJie avatar Mar 28 '22 09:03 crossoverJie

Calling ReconsumeLater without a DLQ policy doesn't make sense in most cases. that sounds some messages could be stuck in an infinite loop forever.

shileiyu avatar Mar 29 '22 04:03 shileiyu

@shileiyu Yes, I agree.

However, there may be careless developers like me who cause the program to run with panic which can even affect the business, this may cut some losses.

crossoverJie avatar Mar 29 '22 06:03 crossoverJie

I’m thinking how could we have a better interface to help developers actively avoiding the pitfall.

shileiyu avatar Mar 30 '22 06:03 shileiyu

@crossoverJie do you think having a default DLQ policy would be a better solution to this issue?

shileiyu avatar Apr 01 '22 08:04 shileiyu

@shileiyu It's a good idea. If RetryEnable==true in the Java SDK, a default policy is created. image

crossoverJie avatar Apr 09 '22 17:04 crossoverJie

@shileiyu It's a good idea. If RetryEnable==true in the Java SDK, a default policy is created. image

In Go SDK, we have also made the same settings. When retry is enabled, a default DLQ policy will be created. The essential reason for this panic is that we have not enabled retry, but are trying to use ReconsumeLater

if options.DLQ == nil {
			options.DLQ = &DLQPolicy{
				MaxDeliveries:    MaxReconsumeTimes,
				DeadLetterTopic:  dlqTopic,
				RetryLetterTopic: retryTopic,
			}
		}

wolfstudy avatar Apr 13 '22 03:04 wolfstudy