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

[feature] Max Retry per msg feature added #939

Closed
wants to merge 4 commits into from

Conversation

ngoyal16
Copy link
Contributor

@ngoyal16 ngoyal16 commented Jan 4, 2023

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #890

Motivation

Add a feature to allow Max Retry per msg.

Modifications

Msg will have max reconsume times property. if not present use default property from consumer max deliveries

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added Integration tests for per msg retry login

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: no
  • The wire protocol: (yes / no)

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Signed-off-by: Nitin Goyal <[email protected]>
Signed-off-by: Nitin Goyal <[email protected]>
@nodece nodece requested review from nodece and RobertIndie January 4, 2023 08:22
@nodece
Copy link
Member

nodece commented Jan 4, 2023

It looks like you want to use the Properties to override the MaxDeliveries of dlq.

@ngoyal16
Copy link
Contributor Author

ngoyal16 commented Jan 4, 2023

It looks like you want to use the Properties to override the MaxDeliveries of dlq.

This is the simplest and best way I found for it.. because retry and dlq is managed at client side in all libs so we can use the same...

@nodece
Copy link
Member

nodece commented Jan 4, 2023

It looks like you want to use the Properties to override the MaxDeliveries of dlq.

This is the simplest and best way I found for it.. because retry and dlq is managed at client side in all libs so we can use the same...

I see. I suggest you make a discussion to mailing lists, see https://pulsar.apache.org/community/#section-discussions.

Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

Thanks @ngoyal16 work for this. Sorry for request change.

Currently, we prefer users to use the nack backoff Policy to customize their own retry strategy. Currently NackBackoff is an interface that provides a default backoff retry strategy. If the strategy does not meet the needs, we can try to implement the NackBackoff interface to meet the current needs.

The current implementation of ReconsumeLater relies on the function of delaying messages, and the Go SDK still does not support the backoff retry function. Of course, it is not planned to continue to support it in the future. The following problems are found in actual production use:

  1. Too many delayed messages will increase the load on Broker
  2. NackBackoff can completely replace the usage logic of ReconsumeLater

If there is something unsatisfactory, we prefer to support more customized features in NackBackoff

@nodece
Copy link
Member

nodece commented Mar 14, 2023

Close this PR, this idea has not been approved by PMC.

@nodece nodece closed this Mar 14, 2023
@jcass8695
Copy link

@nodece what is PMC, and why was it not approved? I created an issue today (#1167) which I think this PR would have addressed.

@RobertIndie
Copy link
Member

@nodece what is PMC, and why was it not approved? I created an issue today (#1167) which I think this PR would have addressed.

The discussion happened here: https://lists.apache.org/thread/b9rfv6t307z439xx3zt2ym4p140qzp06
For the term of PMC, please see: https://pulsar.apache.org/community/#section-governance

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.

[feature request] - How can we set retry logic per msg
5 participants