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

Uses old Nats. #381

Open
gedw99 opened this issue Aug 24, 2023 · 6 comments
Open

Uses old Nats. #381

gedw99 opened this issue Aug 24, 2023 · 6 comments

Comments

@gedw99
Copy link

gedw99 commented Aug 24, 2023

https://github.com/ThreeDotsLabs/watermill/tree/master/_examples/real-world-examples/server-sent-events

Needs upgrading to new nats jetstream public api: https://github.com/nats-io/nats.go/tree/main/jetstream

I have not checked what other examples use the old nats streaming yet

@gedw99 gedw99 changed the title https://github.com/ThreeDotsLabs/watermill/tree/master/_examples/real-world-examples/server-sent-events Uses old Nats Uses old Nats. Aug 24, 2023
@gedw99
Copy link
Author

gedw99 commented Aug 24, 2023

https://github.com/ThreeDotsLabs/watermill-nats/tree/master/pkg/nats Use jetstream but not the new public api

it has a stable api and also does things like ensure a topic / stream exists as well as better ordering guarantees

@AlexCuse
Copy link
Contributor

AlexCuse commented Sep 3, 2023

I have an issue open right now to support the new API. #373

Right now I am thinking it would make the most sense to introduce a whole new package in /pkg/jetstream that uses the new API under the covers. This would be a good opportunity to simplify configuration etc as whats in the nats pkg is largely ported from the old STAN implementation.

Maybe it would be better to try and get the jetstream package added first and use upgrading the example as a test bed to suss out pain points etc....

@AlexCuse
Copy link
Contributor

AlexCuse commented Sep 8, 2023

Possibly of interest @gedw99 - ThreeDotsLabs/watermill-nats#13

I hope to have this passing tests in the not too distant future. But it is good enough to run the new example as is.

@gedw99
Copy link
Author

gedw99 commented Feb 1, 2024

hey @AlexCuse Your PR was merged and tagged at https://github.com/ThreeDotsLabs/watermill-nats/releases/tag/v2.0.2, so we can close this issue ?

@AlexCuse
Copy link
Contributor

AlexCuse commented Feb 1, 2024

No I don't think so @gedw99 - the example you mention is still on the old STAN-based v1.0.7 and would need some updates to the way publishers and subscribers are bootstrapped to use the v2 api (whether using nats or jetstream package).

Additionally its probably OK for use in an example but I would still consider the jetstream package experimental. There is a tricky race condition I have been chasing for awhile when I have time. It happens on almost every run in CI but is harder to replicate locally on decently spec'd hardware. I find if I run go clean -testcache && make jetstream_test_race repeatedly I do get failures about on about 1/3 runs though. They are not consistent though, sometimes the run fails the race detector and other times its a panic for send on closed channel in the message handler. I've found a few issues that seem like they should fix it but I still end up with callbacks arriving late to the handler and making it through to the output channel send.

I am hoping the new Drain functionality added to the nats client package in v1.32.0 will provide a path forward but I have not found it just yet. Sometimes I think it would be easier to just jump ahead to batching pull consumers since that would provide more control and that was what I really wanted the new package to support for maximal throughput, but the configuration changes a lot and I do think supporting Consume well first is probably the right incremental step.

@gedw99
Copy link
Author

gedw99 commented Feb 1, 2024

thanks for the explanation @AlexCuse

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

No branches or pull requests

2 participants