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 crude first version of DLQ #97

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GeroL
Copy link

@GeroL GeroL commented Feb 11, 2022

No description provided.

@GeroL
Copy link
Author

GeroL commented Feb 11, 2022

This is a first version as I do not have any more time this week. Will work on it later again.

@justinwujian
Copy link

Why is this request not processed? Now we are needing to use the functionality of the consumer to handle dead letters.

@GeroL
Copy link
Author

GeroL commented May 5, 2022

I could not finish this yet due to personal time constraints. I try to look into it next week. Would be helpful if this gets some comments on the code

@blankensteiner
Copy link
Contributor

Hi @GeroL
Let's start with a description of the feature before diving into the current implementation.
How should this work? How is DotPulsar made aware that something is "dead"?
What is the difference between a "deadLetterTopic" and "retryLetterTopic"?
How is it decided when a message should go onto one or the other?
Br
db

@jonclare
Copy link

@blankensteiner
My understanding is that this would all be driven from a negative acknowledgement on a message and that the behaviour of the 'retryLetterTopic' and 'deadLetterTopic' is based on what Pulsar supports for each.
https://pulsar.apache.org/docs/en/2.10.0/concepts-messaging/#negative-acknowledgement

From what I can see of what @GeroL has implemented so far, it looks like he's basing the types on what the Java client supports for those concepts.

We need similar concepts for what our team is doing with Pulsar. I'm keen to see this one make some progress.

@blankensteiner
Copy link
Contributor

Hi @jonclare

Ah, I see. Sadly, like so many other features of Pulsar, negative acknowledgment, multi-topic subscription, RetryLetterTopic, and DeadLetterTopic are not first-class citizens in Pulsar, meaning that the protocol (and to a large extent also the broker) is not aware of those concepts. So instead of handling the complexity server-side, every client has to become complex (and also with the consequence of added load on the network and servers) and get the expected behavior spot on. This is a problem because the documentation on the protocol/behavior level is close to non-existent and the user documentation is lacking the depth needed to act as an implementation guide. So, we have to read the Java Client code and/or reverse engineer the protocol to see how things should be implemented, which is a very time-consuming task.

Instead of just copying the Java client, let's have a talk about the wanted feature and see how that could fit into DotPulsar. Generally, I would prefer to build a feature on top of DotPulsar instead of into DotPulsar, if possible.
A consumer has the 'Receive' method to get a single message and we have two extension methods that use this method. The 'Messages' extension method will return an IAsyncEnumerable and the consumer also has the 'Process' extension method which takes a Func to call when a message is received. The latter also supports creating traces and will auto-acknowledge the messages. I think this should be the default way to process messages using DotPulsar and Receive/Messages the more low-level interfaces. I was planning to expand 'Process' so that the user could give some options for a degree of parallelism and other useful stuff and think the retry/dead-letter stuff could be implemented here as well?
The current implementation will catch exceptions from the user's callback Func and add the exception info to the activity and then acknowledge the message. We could inject an exception handler that could send the message to a retry topic before acknowledging it. I guess 'Process' will need to see if there is a retry topic and then create a consumer for it and of course also create producers for the retry topic and the dead letter topic.

Could that be a way to go?

/db

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.

4 participants