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

Proposal to improve eventing/channel/MessageDispatcher #7456

Closed
astelmashenko opened this issue Nov 16, 2023 · 3 comments
Closed

Proposal to improve eventing/channel/MessageDispatcher #7456

astelmashenko opened this issue Nov 16, 2023 · 3 comments
Labels
kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. triage/needs-user-input Issues which are waiting on a response from the reporter

Comments

@astelmashenko
Copy link

Problem
There is knative.dev/eventing golang library, it has channel package. There is MessageDispatcher in channel package. Simplified diagram how it is working:
image

The message dispatcher is used by eventing brokers, like eventing-natss, eventing-kafka (most likely), etc. The problem is in the MessageDispatcher, what if dispatcher crashes? The message should be re-delivered by an underlying message queue/stream, for that message re-delivery should be configured on underlying stream consumer, e.g. in Nats/JetStream case there is MaxDeliver property for a consumer. The same time there are retries configured on trigger/subscription level. It is possible that there will be more re-deliveries then configured on trigger level, image message was retried 4 of 5 times and then dispatcher crashes, after restart the message is redelivered by Nats and it is possible it will be retires another 5 times.
Another thing is why should a MessageDispatcher retry message in-memory if there is a feature on underlying stream consumer like MaxDeliver on JetStream?
If I'd like to utilize JetStream's retry feature, then I need to totally re-implement MessageDispatcher because I do not want to lose DLQ and replyUrl functionality.

Persona:
Contributor/Corporate (employed) maintainer

Exit Criteria
MessageDispatcher has pluggable version of DispatchMessageWithRetries so that it is possible to identify if there are no retries on underlying stream consumer side.

Additional context (optional)
Related PR knative-extensions/eventing-natss#426

@pierDipi
Copy link
Member

pierDipi commented Nov 24, 2023

MessageDispatcher has pluggable version of DispatchMessageWithRetries so that it is possible to identify if there are no retries on underlying stream consumer side.

On main MessageDispatcher has been renamed to Dispatcher with an improved API for new use cases (like #6806).

Given that can we describe how the new Dispatcher API would look like to support your use case?

@pierDipi pierDipi added the triage/needs-user-input Issues which are waiting on a response from the reporter label Nov 24, 2023
@astelmashenko
Copy link
Author

I tried to think about about function's signature, but it is not only signature. The Send function it self should be changed. I'd say it is not necessary to try to make it more generic or accept some tricky interfaces. Instead it'd be better to make it modular, the straght forwad is try to split Send method to something like, SendToTarget, SendToDQL, etc. At least make executeRequest public.
I'll try to think more on that direction.

Copy link

github-actions bot commented Mar 6, 2024

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 6, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. triage/needs-user-input Issues which are waiting on a response from the reporter
Projects
None yet
Development

No branches or pull requests

2 participants