feat: option to query metrics only on polling interval #3940

zroubalik · 2022-11-30T14:54:47Z

Signed-off-by: Zbynek Roubalik [email protected]

Adding an option in the trigger spec, that enables caching metric values during polling interval. Then each request coming to KEDA Metrics Server from k8s server (HPA controller), reads the value from this cache and not querying the external service directly. This reduces load on the external services and user have the ability to select this option for an individual trigger in ScaledObject.

this feature is EXPERIMENTAL and I'd like to gather feedback, before making it a stable one
this feature is available only for ScaledObjects
I named this property ~~queryMetricsOnPollingInterval~~, though I am not happy with the name and I am eager to hear suggestions -> useCachedMetrics

See example:

  - type: kafka
    useCachedMetrics: true     # <-- NEW OPTION, default value: false
    metadata:
      bootstrapServers: kafka.svc:9092
      consumerGroup: my-group
      topic: test-topic
      lagThreshold: '5'

Code changes:

I merged scaler.IsActive() and scaler.GetMetrics() calls to single method scaler.GetMetricsAndActivity() -> this way we don't ask for the same information twice. I was able to reduce this calls for majority of scalers, there are some leftovers to be refactored, as the logic was more complex (so in scaler.GetMetricsAndActivity() we call scaler.IsActive() and scaler.GetMetrics() methods instead of single call) - TODO tracking issue for leftovers
refactored scalehandler and scalerscache a little bit

Outstanding issues:

How to name this property?
e2e test
more unit tests
print warning message if this message is being used in ScaledJobs - it doesn't work there
pring error if this is used by scaler where it doesn't make sense - cpu, memory, cron Where else?

Checklist

Commits are signed with Developer Certificate of Origin (DCO - learn more)
Tests have been added
A PR is opened to update the documentation on trigger.name & trigger.useCachedMetrics docs keda-docs#1003
Changelog has been updated and is aligned with our changelog requirements

Fixes #3921

Relates to #2282

zroubalik · 2022-11-30T15:27:49Z

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

zroubalik · 2022-12-01T22:35:34Z

/run-e2e

Update: You can check the progress here

zroubalik · 2022-12-01T22:35:40Z

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

zroubalik · 2022-12-02T09:30:10Z

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

zroubalik · 2022-12-02T10:03:52Z

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

zroubalik · 2022-12-02T13:38:25Z

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

zroubalik · 2022-12-02T21:53:29Z

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

zroubalik · 2022-12-02T21:53:38Z

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

zroubalik · 2022-12-03T13:06:46Z

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

zroubalik · 2022-12-03T13:49:19Z

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

zroubalik · 2022-12-03T15:00:41Z

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

zroubalik · 2022-12-06T14:14:36Z

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

JorTurFer

Awesome work! I have left some comments inline

CREATE-NEW-SCALER.md

Makefile

controllers/keda/scaledobject_controller.go

pkg/scalers/azure_eventhub_scaler.go

pkg/scalers/external_scaler.go

pkg/scalers/predictkube_scaler.go

pkg/scaling/scale_handler.go

tests/internals/cache_metrics/cache_metrics_test.go

zroubalik · 2022-12-06T17:57:27Z

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

zroubalik · 2022-12-06T19:09:11Z

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

v-shenoy · 2022-12-06T20:28:12Z

@zroubalik I took a look at the PR, but it's kind of hard for me to understand the changes with 92 files changed (although I guess most of those are just merging the IsActive & GetMetrics methods for the scalers).

I am also a little bit confused by the PR title. Doesn't KEDA already query metrics on polling interval?

JorTurFer

LGTM! 2 suggestions inline, but not blockers

tests/internals/cache_metrics/cache_metrics_test.go

tomkerkhove · 2022-12-07T07:11:21Z

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

champly · 2022-12-07T07:51:46Z

@champly could you please help me understand why is this test failing?

• Failure [5.076 seconds]
ScaledObjectController
/__w/keda/keda/controllers/keda/scaledobject_controller_test.go:54
  scaleobject ready condition 'False/Unknow' to 'True' will requeue [It]
  /__w/keda/keda/controllers/keda/scaledobject_controller_test.go:721

  Timed out after 5.000s.
  Expected
      <v1.ConditionStatus>: True
  To satisfy at least one of these matchers: [%!s(*matchers.EqualMatcher=&{False}) %!s(*matchers.EqualMatcher=&{Unknown})]

you wrote the test and I am not sure, what could break the behavior, thanks!

On my local test it looks like it should work will.

zroubalik · 2022-12-07T12:41:02Z

@champly could you please help me understand why is this test failing?

• Failure [5.076 seconds]
ScaledObjectController
/__w/keda/keda/controllers/keda/scaledobject_controller_test.go:54
  scaleobject ready condition 'False/Unknow' to 'True' will requeue [It]
  /__w/keda/keda/controllers/keda/scaledobject_controller_test.go:721

  Timed out after 5.000s.
  Expected
      <v1.ConditionStatus>: True
  To satisfy at least one of these matchers: [%!s(*matchers.EqualMatcher=&{False}) %!s(*matchers.EqualMatcher=&{Unknown})]

you wrote the test and I am not sure, what could break the behavior, thanks!

On my local test it looks like it should work will.

@champly thanks a lot for the response, I have already solved this :)

zroubalik · 2022-12-07T13:25:40Z

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

zroubalik · 2022-12-07T22:03:59Z

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

zroubalik · 2022-12-07T22:26:17Z

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

zroubalik · 2022-12-07T22:27:00Z

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

zroubalik · 2022-12-08T06:34:12Z

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

tomkerkhove · 2022-12-08T08:25:02Z

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

tomkerkhove · 2022-12-08T08:25:13Z

Last run and I'll merge it @zroubalik

zroubalik · 2022-12-08T09:57:03Z

Last run and I'll merge it @zroubalik

please let the merge on me, there might be some PRs merged before this one, so rebase might be needed.

Signed-off-by: Zbynek Roubalik <[email protected]>

zroubalik · 2022-12-08T17:04:37Z

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

zroubalik · 2022-12-08T17:05:09Z

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

tomkerkhove · 2022-12-08T19:23:31Z

/run-e2e

Update: You can check the progress here

zroubalik force-pushed the cache branch from 10c6543 to 2e4152e Compare November 30, 2022 15:04

zroubalik force-pushed the cache branch from 2e4152e to dec8d9c Compare December 1, 2022 22:34

zroubalik force-pushed the cache branch from dec8d9c to dc764d4 Compare December 2, 2022 10:02

zroubalik force-pushed the cache branch 2 times, most recently from f29ecf9 to 829a541 Compare December 2, 2022 13:38

zroubalik marked this pull request as ready for review December 2, 2022 13:39

zroubalik requested a review from a team as a code owner December 2, 2022 13:39

zroubalik changed the title ~~[WIP] cache requests~~ feat: option to query metrics only on polling interval Dec 2, 2022

zroubalik force-pushed the cache branch from 829a541 to 74f2465 Compare December 2, 2022 13:59

zroubalik requested a review from v-shenoy December 2, 2022 13:59

zroubalik marked this pull request as draft December 2, 2022 16:08

zroubalik force-pushed the cache branch from 74f2465 to 928a38e Compare December 2, 2022 21:28

zroubalik force-pushed the cache branch 2 times, most recently from 1d0879c to 3795e1b Compare December 3, 2022 12:25

zroubalik force-pushed the cache branch 3 times, most recently from 48c351c to b0f48bc Compare December 3, 2022 13:49

zroubalik force-pushed the cache branch 2 times, most recently from 47285f9 to cd1f4dc Compare December 3, 2022 14:59

JorTurFer reviewed Dec 6, 2022

View reviewed changes

zroubalik force-pushed the cache branch from fc3611e to c9038bc Compare December 6, 2022 17:57

zroubalik force-pushed the cache branch 2 times, most recently from 4ee7c42 to 0960d3e Compare December 6, 2022 19:09

JorTurFer approved these changes Dec 6, 2022

View reviewed changes

tests/internals/cache_metrics/cache_metrics_test.go Show resolved Hide resolved

tests/internals/cache_metrics/cache_metrics_test.go Show resolved Hide resolved

zroubalik force-pushed the cache branch from 0960d3e to 3e72aad Compare December 7, 2022 21:29

zroubalik force-pushed the cache branch from 3e72aad to 52dfb31 Compare December 7, 2022 22:25

JorTurFer approved these changes Dec 8, 2022

View reviewed changes

zroubalik force-pushed the cache branch from 52dfb31 to 7c09ece Compare December 8, 2022 17:00

feat: option to query metrics only on polling interval

a2a81e3

Signed-off-by: Zbynek Roubalik <[email protected]>

zroubalik force-pushed the cache branch from 7c09ece to a2a81e3 Compare December 8, 2022 17:03

zroubalik merged commit c93a7d4 into kedacore:main Dec 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: option to query metrics only on polling interval #3940

feat: option to query metrics only on polling interval #3940

zroubalik commented Nov 30, 2022 •

edited

Loading

zroubalik commented Nov 30, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

JorTurFer left a comment

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

v-shenoy commented Dec 6, 2022

JorTurFer left a comment

tomkerkhove commented Dec 7, 2022 •

edited by github-actions bot

Loading

champly commented Dec 7, 2022

zroubalik commented Dec 7, 2022

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 8, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 8, 2022

zroubalik commented Dec 8, 2022

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 8, 2022 •

edited by github-actions bot

Loading

feat: option to query metrics only on polling interval #3940

feat: option to query metrics only on polling interval #3940

Conversation

zroubalik commented Nov 30, 2022 • edited Loading

Outstanding issues:

Checklist

zroubalik commented Nov 30, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 1, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 1, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 2, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 2, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 2, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 2, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 2, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 3, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 3, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 3, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 6, 2022 • edited by github-actions bot Loading

JorTurFer left a comment

Choose a reason for hiding this comment

zroubalik commented Dec 6, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 6, 2022 • edited by github-actions bot Loading

v-shenoy commented Dec 6, 2022

JorTurFer left a comment

Choose a reason for hiding this comment

tomkerkhove commented Dec 7, 2022 • edited by github-actions bot Loading

champly commented Dec 7, 2022

zroubalik commented Dec 7, 2022

zroubalik commented Dec 7, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 7, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 7, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 7, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 8, 2022 • edited by github-actions bot Loading

tomkerkhove commented Dec 8, 2022 • edited by github-actions bot Loading

tomkerkhove commented Dec 8, 2022

zroubalik commented Dec 8, 2022

zroubalik commented Dec 8, 2022 • edited by github-actions bot Loading

zroubalik commented Dec 8, 2022 • edited by github-actions bot Loading

tomkerkhove commented Dec 8, 2022 • edited by github-actions bot Loading

zroubalik commented Nov 30, 2022 •

edited

Loading

zroubalik commented Nov 30, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 2, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 3, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 6, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 7, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 8, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

zroubalik commented Dec 8, 2022 •

edited by github-actions bot

Loading

tomkerkhove commented Dec 8, 2022 •

edited by github-actions bot

Loading