apache / apache/pulsar

[Go Functions] deadLetterTopic and maxMessageRetries are carried into the instance but never applied

Open
#26,406 0 comments 0 reactions 0 assignees View on GitHub
area/function type/bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.