watermill icon indicating copy to clipboard operation
watermill copied to clipboard

Uses old Nats.

Open gedw99 opened this issue 2 years ago • 6 comments

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 avatar Aug 24 '23 11:08 gedw99

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

gedw99 avatar Aug 24 '23 11:08 gedw99

I have an issue open right now to support the new API. https://github.com/ThreeDotsLabs/watermill/issues/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 avatar Sep 03 '23 21:09 AlexCuse

Possibly of interest @gedw99 - https://github.com/ThreeDotsLabs/watermill-nats/pull/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.

AlexCuse avatar Sep 08 '23 15:09 AlexCuse

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 ?

gedw99 avatar Feb 01 '24 12:02 gedw99

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.

AlexCuse avatar Feb 01 '24 14:02 AlexCuse

thanks for the explanation @AlexCuse

gedw99 avatar Feb 01 '24 14:02 gedw99