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

Add discard as an option for custom recoverability policies #5327

Merged
merged 6 commits into from
Mar 19, 2019

Conversation

danielmarbach
Copy link
Contributor

@danielmarbach danielmarbach commented Feb 14, 2019

Allows discarding a message in the custom recoverability policy.

More information can be found under

https://docs.particular.net/nservicebus/recoverability/custom-recoverability-policy#implement-a-custom-policy-full-customization

Orignal context On a live chat today a user had the following requirement

They have certain types of messages that they want to retry a few times and after they failed a certain number of times they want to ignore/discard the message. We first started exploring whether it is possible to implement this scenario with the recoverability policy but found out it is not possible.

Trying to implement that with behaviors in the pipeline becomes really cumbersome because you are essentially faced with reimplementing parts of the recoverability seam inside behaviors. Especially the counting of how many times a message has failed can become very brittle.

This demonstrates that a discard action in the recoverability seam would be easy to add in a non-breaking way and could provide good benefit to some of our customers.

There is a workaround that we came up with for the customer. You can currently implement a MoveToError action and move it to a queue that exists (but not the error queue) and then have a no-op handler on that queue or from time to time purge that queue. But that requires you to deploy an additional endpoint that listens on that queue or have a dedicated purge this queue job that runs from time to time (plus obviously the unnecessary additional infrastructure resources that are required).

@danielmarbach danielmarbach requested a review from a team February 14, 2019 09:05
@gardang
Copy link

gardang commented Feb 14, 2019

This would be a great feature for us and would obviously make the code more readable on this types of scenarios. So this functionality would be greatly appreciated =)

Co-Authored-By: danielmarbach <[email protected]>
andreasohlund
andreasohlund previously approved these changes Feb 14, 2019
Copy link
Member

@bording bording left a comment

Choose a reason for hiding this comment

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

Are we sure we want to add a features that is basically allowing message loss?

@timbussmann
Copy link
Contributor

Are we sure we want to add a features that is basically allowing message loss?

I'm fine allowing this option in a very advanced API where you clearly have to opt-in for discarding. We also allow message loss when using non-transactional transport configuration.

@HEskandari HEskandari self-assigned this Feb 20, 2019
@bording
Copy link
Member

bording commented Feb 22, 2019

I think this is ready to remove the WIP?

@HEskandari
Copy link
Contributor

Is this done? anything left here other than removing the WIP and merging?

@danielmarbach
Copy link
Contributor Author

All done

@HEskandari HEskandari changed the title WIP Implement discard action for recoverability policy Implement discard action for recoverability policy Mar 19, 2019
@HEskandari
Copy link
Contributor

@danielmarbach I have removed the WIP from the PR. Let's merge this and release it so I can get the doco in as well.

@andreasohlund andreasohlund merged commit c16218d into develop Mar 19, 2019
@andreasohlund
Copy link
Member

We need to figure out how to make sure annoucement text is available as well when releasing. Perhaps a draft 7.2.0 announcement?

@danielmarbach danielmarbach added this to the 7.2.0 milestone Mar 19, 2019
@danielmarbach danielmarbach deleted the discard-action branch March 19, 2019 07:32
@danielmarbach danielmarbach changed the title Implement discard action for recoverability policy Add discard as an option for custom recoverability policies Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants