[Go Functions] deadLetterTopic and maxMessageRetries are carried into the instance but never applied
- Dominant language
- Java
- Stars
- 15.3k
- Forks
- 3.8k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 160
Description
Master Issue: #26404
### Search before reporting
- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. #26397 is the same defect in the **Python** runtime, with a fix open at #26400; this is the Go counterpart.
### Motivation
The Go runtime builds `RetryDetails` into its own `FunctionDetails` from the instance configuration:
```go
// pulsar-function-go/pf/instanceConf.go:115-118
RetryDetails: &pb.RetryDetails{
MaxMessageRetries: cfg.MaxMessageRetries,
DeadLetterTopic: cfg.DeadLetterTopic,
},
```
and then never reads it. `git grep -in "DLQ\|DeadLetter\|MaxRedeliver" -- 'pulsar-function-go/pf/*.go'` returns only that one construction site. No `pulsar.DLQPolicy` is ever built, and `setupConsumer` passes no DLQ options to `Subscribe`.
The configuration is therefore accepted end to end and has no effect: a Go function created with `--dead-letter-topic` and `--max-message-retries` reports both back from `functions get`, and messages that fail repeatedly are redelivered forever instead of being routed to the dead letter topic. The Java runtime applies them in `PulsarSource` (guarded by `hasRetryDetails()` in `JavaInstanceRunnable`).
Populating the field and never consuming it is worse than not carrying it at all — it makes the gap invisible to anyone reading the instance configuration.
### Solution
Build a `pulsar.DLQPolicy` from `funcDetails.RetryDetails` in `setupConsumer` and pass it in the `ConsumerOptions`. The Go client supports this:
```go
type DLQPolicy struct {
MaxDeliveries uint32
DeadLetterTopic string
RetryLetterTopic string
InitialSubscriptionName string
}
```
Points worth settling, mirroring the ones raised on the Python fix in #26400:
1. **Presence check.** `RetryDetails` is a message field, so `funcDetails.GetRetryDetails() != nil` is the equivalent of Java's `hasRetryDetails()`. A scalar check on `MaxMessageRetries` cannot distinguish unset from zero.
2. **`maxMessageRetries` of 0.** Java accepts `>= 0`. Whether the Go client's `MaxDeliveries` accepts 0 meaningfully should be confirmed rather than assumed — the Python client rejects a redelivery count below 1, which is why #26400 attaches no policy in that case and warns.
3. **Empty `deadLetterTopic`.** Java sets the topic only when non-empty, leaving the client to derive `--DLQ`. Passing an empty string through would override that with an invalid name.
4. **Subscription type.** A dead letter policy only takes effect on Shared or KeyShared. That interacts with the sibling issue #26405, where `retainOrdering` currently selects nothing and `retainKeyOrdering` is ignored; whichever lands second should handle the combination rather than silently producing an ineffective policy.
### Alternatives
Removing the `RetryDetails` construction so the configuration is visibly absent would be honest but strictly less useful — the fields are already plumbed, and the Go client supports the feature.
### Anything else?
Verified against `origin/master`.
### Are you willing to submit a PR?
- [X] I'm willing to submit a PR!
Contributor guide
Research direction
Start in pulsar-function-go/pf/instanceConf.go and locate setupConsumer; run the issue's git grep to confirm RetryDetails is only constructed and never consumed. Read the Go client's DLQPolicy and the Java runtime's PulsarSource handling, then account for the subscription-type interaction with sibling issue #26405. Done means configured retry and dead-letter settings affect failed-message delivery without overriding empty-topic defaults.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100