JetStream scaler should query the stream consumer leader when clustered

[rayjanoka]

I went to test drive the new JetStream scaler on my project and it started to have an issue. Eventually I figured out that the NATS monitoring endpoint isn't reporting accurate metrics on stream consumers from all pods in a cluster.

I found that when running a cluster of nats pods, only the stream consumer's leader pod reports the accurate number of messages in the queue. The jetstream scaler would work fine for me at first as long as keda was connected to the nats consumer leader pod, but as soon as I bounced keda's connection to a nats consumer replica pod the num_pending value there counts up but never down, so my deployment just scales up and up and up.

➜ nats consumer info test-stream durable

Cluster Information:
                Name: nats
              Leader: nats-1   <---- Leader Pod
             Replica: nats-0, current, seen 0.33s ago
             Replica: nats-2, current, seen 0.33s ago

# Consumer Leader (accurate)
➜ curl -s "http://nats-1.nats.nats.svc.cluster.local:8222/jsz?consumers=true" | grep num_pending
              "num_pending": 0,

# Replicas (counting up only)
➜ curl -s "http://nats-0.nats.nats.svc.cluster.local:8222/jsz?consumers=true" | grep num_pending
              "num_pending": 6,
➜ curl -s "http://nats-2.nats.nats.svc.cluster.local:8222/jsz?consumers=true" | grep num_pending
              "num_pending": 6,

We are able to discover the jetstream consumer's leader via the existing monitoring endpoint call.

$ curl -s "http://nats.nats.svc.cluster.local:8222/jsz?consumers=true" | jq '.account_details[0].stream_detail[0].consumer_detail[0].cluster.leader'
"nats-0"

To get the accurate count to the scaler I wrote a change to make a 2nd request directly to that consumer leader pod via the headless svc, ex consumer-leader-pod.nats.nats.svc.cluster.local.

NOTE: This fix will only work for clusters who have the same number of pods as stream replicas, ex. 3 pods and 3 stream replicas or 5 pods and 5 stream replicas.

➜ nats stream info test-stream
Information for Stream test-stream

Configuration:

             Subjects: sub
             Replicas: 3   <---- stream replica setting here

this is resolved below #3564 (comment)
If there are less stream replicas than pods, ex. 3 pods and only 1 stream replica, NATS will only display metrics for that consumer on the single pod that the stream is assigned to, the other pods have no record of the consumer at all so it will only find the metric if the k8s svc happens to terminate you to that particular pod. Without any other clues to figure out which pod has the metric I think we'd have to rotate through each NATS pod blindly until we found the pod, not great. (I can document this limitation in the keda-docs for now)

@goku321 for visibility - thanks for contributing this!

I'll send a ticket over to the NATS side as well and see if they can work to provide accurate metrics across all NATS servers in a cluster so no matter what server we connect to we see everything.

Checklist

• Commits are signed with Developer Certificate of Origin (DCO - learn more)
• Tests have been added
• [NA] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
• [NA] A PR is opened to update the documentation on (repo) (if applicable)
• Changelog has been updated and is aligned with our changelog requirements

Fixes #3860

Relates to #

[goku321]

@rayjanoka Thank you so much for giving it a test drive! Really appreciate all your efforts :)

Now, I'm thinking if we really need the single server JetStream tests and instead add tests for clustered JetStream. What do you think?

[rayjanoka]

@rayjanoka Thank you so much for giving it a test drive! Really appreciate all your efforts :)

Now, I'm thinking if we really need the single server JetStream tests and instead add tests for clustered JetStream. What do you think?

@goku321 nice! I did notice we had separate tests like that for redis and I think it is probably a good idea here as well. I don't have much experience writing tests, and it might take me a while to circle back to this to give those a shot. If you or anyone else would like to take a crack at it that would be fine.

[goku321]

@rayjanoka Sure. I can start with the tests in a week or so. I hope that's okay :)

[rayjanoka]

I'm back on this, I'm going to work on putting this in a function and writing some tests.

[rayjanoka]

ok, this is looking good! I did some serious surgery and we're at 96% test coverage! 😆

I'm going to give this a run in my lab and I'll let ya know how it goes.

[rayjanoka]

I think this is ready for review, it tested ok in my environment.

• De-duplicated code
• Lots of new tests
• All tests passed!
• Coverage:

Let me know if I need to do anything else!

[zroubalik]

This change is looking good, there are a few leftovers:

• could you please create a new issue describing the problem and reference it in the changelog?
• the test coverage is great! I think that @goku321 was talking about e2e test that we have, to support clustered setup: Provide scaler for NATS Jetstream #2391

[zroubalik]

/run-e2e nats*
Update: You can check the progress here

[rayjanoka]

Updates

• new clustered e2e test with 3 nats nodes - takes about 6.5 mins to run
  • tests all combinations of stream replicas (1, 2, & 3) in the cluster
• lowered the number of messages to process in the standalone test so it is faster
• adding all combinations of streams replicas to the e2e test required supporting all the stream replicas. This required a bit more logic back in the main code to fall back to scanning each nats node to blindly discover the leader node when sometimes no stream/consumer context is returned in the initial request to nats (when the response is from a node where the stream is not running).
• we are caching the leader once we find it so we only need to make a single request, there are multiple calls to discover in the first run, or if the stream leader moves to another node

Am I able to trigger the e2e test here? or if one of the crew could start it we can see if it works here.

[JorTurFer]

/run-e2e nats*
Update: You can check the progress here

[JorTurFer]

Am I able to trigger the e2e test here? or if one of the crew could start it we can see if it works here.

No you can't. To trigger them, it's required to be in a specific team in the org. I have already triggered

[rayjanoka]

ok, it looks like it worked...I can't see the log, but I see a green check.

I'll take one more look over everything on Monday, then we can do the review and get this finished. thanks!

[JorTurFer]

You can find the specific workflow logs following the link in the comment

There, you have to go to the Execute e2e tests job:

And there you only need to check the step:

[JorTurFer]

I'll take one more look over everything on Monday, then we can do the review and get this finished. thanks!

Perfect! Thanks for your contribution!

[rayjanoka]

You can find the specific workflow logs following the link in the comment

ah hah! of course, the log looks good good!

[rayjanoka]

ok, I just rebased, should be good for review and delivery! @goku321 @JorTurFer @zroubalik

[JorTurFer]

/run-e2e nats*
Update: You can check the progress here

[rayjanoka]

/run-e2e nats* Update: You can check the progress here

LGTM!

the clustered test is long, but I helped a bit overall by cutting 2.5 mins off the standalone run by pushing less messages through in the test, 1000 -> 300.

standalone e2e test: 59 secs
clustered e2e test: 7.3 mins

[tomkerkhove]

/run-e2e nats*
Update: You can check the progress here

[tomkerkhove]

@JorTurFer @zroubalik Can you re-review this PR please to see if we need to change things before our release on Thursday?
@rayjanoka can you fix the merge conflict please?

[rayjanoka]

@JorTurFer @zroubalik Can you re-review this PR please to see if we need to change things before our release on Thursday? @rayjanoka can you fix the merge conflict please?

fixed! thanks @tomkerkhove!

[zroubalik]

/run-e2e nats*
Update: You can check the progress here

[zroubalik]

LGTM

thanks a lot for the contribution!