apache / pulsar

Apache Pulsar - distributed pub-sub messaging system
https://pulsar.apache.org/
Apache License 2.0
14.27k stars 3.59k forks source link

[fn][Go] Go functions should support retry/dlq #20515

Open flowchartsman opened 1 year ago

flowchartsman commented 1 year ago

Search before asking

Motivation

Go Functions do not support message retry or DLQ settings. Since the client has support for these and since they are already transported to the runtime, this should be a no-brainer.

Solution

Use the transported DLQ/retry settings to bring the go functions in-line with the java ones

Alternatives

The only alternative at this time is to detect retries automatically by inspecting messages manually and using the (not so great) context.NewOutputMessage API to manually to dispatch messages to the retry or DLQ topics.

Anything else?

No response

Are you willing to submit a PR?

michaeljmarshall commented 1 year ago

Use the transported DLQ/retry settings to bring the go functions in-line with the java ones

Can you clarify this point @flowchartsman? What specific feature is missing in the go sdk? From what I see, there is only the option to configure the max message retires and the DLQ topic. This is true for all function SDKs and is represented by this proto def:

https://github.com/apache/pulsar/blob/e0098ee0c34643371f53ee921bdc361e163f6387/pulsar-functions/proto/src/main/proto/Function.proto#L58-L61

That are represented in the go instance config:

https://github.com/apache/pulsar/blob/510744c20d0339c4b29366f07bf1849cabb81dec/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/go/GoInstanceConfig.java#L82-L83

Thanks!

flowchartsman commented 1 year ago

Sorry, what I mean to say is that it is not enabled client-side and that, while the settings may be transported correctly, there is an explicit check that forbids go functions attempting to use retry. Which is odd, considering that it's client-side.

github-actions[bot] commented 1 year ago

The issue had no activity for 30 days, mark with Stale label.

flowchartsman commented 1 year ago

Just a quick hup to keep this from going stale, as I plan to create a PR for this feature in the coming weeks, and even have a preliminary implementation. Just need some more time and have a couple nagging bugs to fix.

github-actions[bot] commented 1 year ago

The issue had no activity for 30 days, mark with Stale label.