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

Add notes on implementation of js.Publish retries #105

Merged
merged 4 commits into from Mar 21, 2022
Merged

Add notes on implementation of js.Publish retries #105

merged 4 commits into from Mar 21, 2022

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Mar 19, 2022

From base implementation at nats-io/nats.go#930

Signed-off-by: Waldemar Quevedo wally@synadia.com

Signed-off-by: Waldemar Quevedo <wally@synadia.com>
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
adr/ADR-22.md Outdated
## Motivation

When the NATS Server is running with JetStream on cluster mode, there
can be occasional blips in leadership due which can result in a number
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: leadership due which

```go
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
_, err := js.Publish("foo", []byte("bar"), nats.Context(ctx), nats.RetryWait(250*time.Millisecond), nats.RetryAttempts(-1))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that this is applicable only to "context" feature?

What I mean by that is that without a context, the Publish() is still subject to the js "timeout" or "wait" timeout (that all internal APIs requests are subject to, using it as the timeout of the Request() call). So the question is: should libraries without a context still apply the same logic but use the request timeout as the upper limit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will mention that in the base Go client this would also work without context since the ttl of the next request is subtracted the back off time, so the original deadline of the request caps the max duration of the Publish.

adr/ADR-22.md Outdated Show resolved Hide resolved
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wallyqs wallyqs merged commit 51ca8f0 into main Mar 21, 2022
@wallyqs wallyqs deleted the js-retry branch March 21, 2022 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants