nats-io / nats.go

Golang client for NATS, the cloud native messaging system.
https://nats.io
Apache License 2.0
5.34k stars 677 forks source link

Publish RetryAttempt and RetryWait not working as expected #1664

Open sourabhaggrawal opened 1 month ago

sourabhaggrawal commented 1 month ago

Observed behavior

js.Publish("test", []byte("hi"), nats.RetryWait(5*time.Second), nats.RetryAttempts(10)) It gives up at first attempt itself after waiting for 5 second.

Expected behavior

This supposed to retry 10 times if there is "no responder" error and should retry 5 times before it gives up. So after 50 seconds it should give up if there is "no responder" error on every attempt.

Server and client version

nats-server 2.10.3 nats-client 1.35.0

Host environment

Mac OS

Steps to reproduce

Publish a message to subject which is not associated to stream, configure retry attempt and retrywait.

js.Publish("test", []byte("hi"), nats.RetryWait(5*time.Second), nats.RetryAttempts(10))

souravagrawal commented 3 weeks ago

Raised a PR for same here https://github.com/nats-io/nats.go/pull/1667

Jarema commented 3 weeks ago

The current bahavior is restrained by TTL that can be set by nats.AckWait(). The number of retries defines the upper bound for number of retries. TTL still applies.

souravagrawal commented 3 weeks ago

Any specific reason why it only retries in case of "No Responder" case. There can be many other cases too where user expect nats to retry publish. Also this retry is not there in AsyncPublish method of go, however it is there in .net client https://github.com/nats-io/nats.net.v2/blob/ccc212f764508f11a3154ade8d29bfdbb772de02/src/NATS.Client.JetStream/NatsJSContext.cs#L120

Jarema commented 3 weeks ago

Only retrying on no responders is safe. Retrying on Timeout can lead to duplicate messages in the stream, as the operation can finish after client timeout window.

.net async publish is a publish that waits for the ack. The async part comes from the idiom in .net, meaning that it's async operation from .net perspective, not NATS idioms perspective.

We're trying to find a better name for async publish, meaning - a publish that does not wait for ack, but returnes a future of a kind ;).

souravagrawal commented 3 weeks ago

What are your thoughts on providing Retry Attempts in Async Publish ?

sourabhaggrawal commented 2 weeks ago

@Jarema Currently async publish does not support retry, is there any plan to fix/add later.

Jarema commented 2 weeks ago

@sourabhaggrawal at least the new jetstream package does support retries on async publish. Not sure about the legacy one. Will defer that one to @piotrpio

sourabhaggrawal commented 2 weeks ago

Great, can you share some example for async Publish using new jetstream api ? TIA