Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka #2621

RamCohen · 2022-02-10T07:44:24Z

When getting the consumer offset of a Kafka topic for which no offset
was committed yet, the scaler returns with -1 instead of 0, which
causes to scale to the maximum number of replicas.

This fix changes the behavior to return in that case either '1' (the default) or '0', depending on a new scaler parameter scaleToZeroOnInvalidOffset which default to 'false'

Also fixed some typos and used interim variables for error strings.

Fixes #2612 #2033

RamCohen · 2022-02-10T09:00:20Z

/run-e2e kafka*

JorTurFer · 2022-02-10T09:28:20Z

/run-e2e kafka*

Hi @RamCohen , only people with write permission can trigger this.
Now we have 2 execution in progress but I'll trigger this after those finish

JorTurFer · 2022-02-10T09:46:20Z

/run-e2e kafka*
Update: You can check the progres here

JorTurFer · 2022-02-28T11:44:57Z

Does this make sense @kedacore/keda-core-contributors ?

zroubalik

Looking good, thanks! Could you please open documentation PR?

zroubalik · 2022-02-28T16:48:26Z

@bpinske @PaulLiang1 PTAL

PaulLiang1 · 2022-03-01T01:02:00Z

@bpinske @PaulLiang1 PTAL

looks good.
also replied on #2612 (comment)

zroubalik · 2022-03-01T11:06:02Z

I wonder whether we can extend the e2e test to cover this scenario? I think that creating a topic without any message should cause the invalid offset problem, right?

zroubalik · 2022-03-14T09:38:57Z

I wonder whether we can extend the e2e test to cover this scenario? I think that creating a topic without any message should cause the invalid offset problem, right?

@RamCohen do you think you can contribute this?

RamCohen · 2022-03-14T10:08:32Z

I'll have a look, but I'm not sure where have we landed with the proposed solution vis-a-vis having a configuration parameter for returning either 0 or 1 and what would be its default

rrmcclymont · 2022-03-14T10:27:23Z

thanks for this pull request! we are having problems in production due to this issue :) do we have an estimate when the new version will be released?

JorTurFer · 2022-03-14T12:24:16Z

thanks for this pull request! we are having problems in production due to this issue :) do we have an estimate when the new version will be released?

hi @rrmcclymont ,
Our plan is to release v2.7 on May, hopefully on May 9th. I guess that this PR will be merged before that date so it should be included on v2.7.
BTW, if you are interested, you can check the content for the next release in our backlog

rrmcclymont · 2022-03-15T09:39:22Z

thanks @JorTurFer glad to see that we will include this fix in the following release :)

RamCohen · 2022-03-30T09:52:54Z

Added tests

pkg/scalers/kafka_scaler.go

zroubalik · 2022-04-01T12:41:44Z

/run-e2e kafka*
Update: You can check the progres here

zroubalik · 2022-04-05T08:01:20Z

/run-e2e kafka*
Update: You can check the progres here

zroubalik · 2022-04-05T08:02:23Z

@RamCohen could please open docs PR for this?

RamCohen · 2022-04-05T08:34:57Z

kedacore/keda-docs#741

zroubalik

LGTM,

@RamCohen so the last bit is Changelog

When getting the consumer offset of a Kafka topic for which no offset was committed yet, the scaler returns with -1 instead of 0, which causes to scale to the maximum number of replicas. Also fixed some typos and used interim variables for error strings. Fixes kedacore#2612 Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

…ffset Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Update to use strimzi operator to v0.23.0 and Kafka 2.6.0 in order to properly work on Kubernetes 1.21 and up due to deprecation of beta CRD api Also refactor common deploy and status checks to use internal methods Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

zroubalik

LGTM, thanks a lot @RamCohen!

zroubalik · 2022-04-07T13:18:58Z

/run-e2e kafka*
Update: You can check the progres here

akshay201 · 2023-06-08T09:09:21Z

@zroubalik What is the expected behaviour now.
If we do not have any consumer group created for a topic and new messages have arrived in topic, in this case I suppose invalid offset will be returned and keda will return 0 or scale to 0 if scaleToZeroOnInvalidOffset is set to true. Doubt is then if there are no consumers present, there would be no consumer group and offset will be invalid always. How is the problem related to this is solved with this so that consumers are scaled up or keda is activated on invalid offset but messages present in topic?

We want to scale consumers to 0 when there are no messages in topic and scale back consumers if there any new messages.

Apologies if I did not understand or if I am missing something here. Thanks.

pnorth1 · 2023-06-08T14:54:04Z

@akshay201 We faced the same problem as you. I can share the main approaches we considered. I'm also curious to hear from others on how they've worked around this.

One option we considered was to configure the ScaledObject to use a custom metrics endpoint. One could implement logic to connect to the kafka brokers and measure consumer group lag and/or available messages and return a metric on the http endpoint that the ScaledObject is pointing to.
The option we ultimately decided to implement was to have a separate process connect to the Kafka brokers and check if a topic had a valid consumer group lag. If there was not a valid consumer group lag (the consumers have not yet committed any offsets), then the process would publish a set of dummy-messages to the topic. Consumers knew how to digest and ignore these no-op messages, but it still allowed the consumers to commit offsets. In this case, we left scaleToZeroOnInvalidOffset to false so that an initial consumer would exist to commit offsets for the dummy messages.

During exploration we wondered if it was possible to manually commit offsets from the consumer's Kafka client, but I believe the broker's rejected the commit since it was trivial (committing to offset 0 for all partitions). We never really determined if this was a product of the kafka client we were using, or a broker setting, or totally unavoidable. So, that could be something quick to try as well.

akshay201 · 2023-06-09T08:48:19Z

@pnorth1 It seems like issue is resolved from the documentation of latest version
https://keda.sh/docs/2.0/scalers/apache-kafka/#new-consumers-and-offset-reset-policy

Did you try new version?

Update is there from 2.x+ version.

If anyone else tried the same I would like to hear if it worked.

RamCohen requested a review from a team as a code owner February 10, 2022 07:44

loicmathieu mentioned this pull request Feb 10, 2022

Kafka scaler reports -2 and scale to maxReplicas #2612

Closed

RamCohen changed the title ~~Return 0 instead of -1 when detecting an invalid offset in Kafka~~ Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka Feb 17, 2022

zroubalik reviewed Feb 28, 2022

View reviewed changes

RamCohen force-pushed the kafka_invalid_offset branch 2 times, most recently from b1042f0 to 35b592a Compare March 30, 2022 09:51

zroubalik reviewed Apr 1, 2022

View reviewed changes

pkg/scalers/kafka_scaler.go Outdated Show resolved Hide resolved

zroubalik approved these changes Apr 5, 2022

View reviewed changes

RamCohen added 5 commits April 5, 2022 14:51

Add a parameter to specify whether to scale to 1 or 0 on an invalid o…

dfb0d57

…ffset Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Fix int64 type

43c9e22

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Remove semicolon

77eda45

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

RamCohen added 4 commits April 5, 2022 14:51

Add tests for new scaleToZeroOnInvalidOffset configuration

a79eb69

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Extract auth metadata parsing to separate method

005859f

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Dont return an error on invalid offset

d49de59

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

Changelog

e9b8e01

Signed-off-by: Ram Cohen <ram.cohen@gmail.com>

RamCohen force-pushed the kafka_invalid_offset branch from 34713b8 to e9b8e01 Compare April 5, 2022 11:56

zroubalik approved these changes Apr 7, 2022

View reviewed changes

zroubalik merged commit d9172a7 into kedacore:main Apr 7, 2022

zroubalik mentioned this pull request Apr 19, 2022

Kafka scaler not scaling to zero when offset is not properly initialized #2033

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka #2621

Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka #2621

RamCohen commented Feb 10, 2022 •

edited

Loading

RamCohen commented Feb 10, 2022

JorTurFer commented Feb 10, 2022

JorTurFer commented Feb 10, 2022 •

edited by github-actions bot

Loading

JorTurFer commented Feb 28, 2022

zroubalik left a comment

zroubalik commented Feb 28, 2022

PaulLiang1 commented Mar 1, 2022

zroubalik commented Mar 1, 2022

zroubalik commented Mar 14, 2022

RamCohen commented Mar 14, 2022

rrmcclymont commented Mar 14, 2022

JorTurFer commented Mar 14, 2022 •

edited

Loading

rrmcclymont commented Mar 15, 2022

RamCohen commented Mar 30, 2022

zroubalik commented Apr 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Apr 5, 2022 •

edited by github-actions bot

Loading

zroubalik commented Apr 5, 2022

RamCohen commented Apr 5, 2022 •

edited

Loading

zroubalik left a comment

zroubalik left a comment

zroubalik commented Apr 7, 2022 •

edited by github-actions bot

Loading

akshay201 commented Jun 8, 2023 •

edited

Loading

pnorth1 commented Jun 8, 2023

akshay201 commented Jun 9, 2023 •

edited

Loading

Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka #2621

Return 1 or 0 instead of -1 when detecting an invalid offset in Kafka #2621

Conversation

RamCohen commented Feb 10, 2022 • edited Loading

RamCohen commented Feb 10, 2022

JorTurFer commented Feb 10, 2022

JorTurFer commented Feb 10, 2022 • edited by github-actions bot Loading

JorTurFer commented Feb 28, 2022

zroubalik left a comment

Choose a reason for hiding this comment

zroubalik commented Feb 28, 2022

PaulLiang1 commented Mar 1, 2022

zroubalik commented Mar 1, 2022

zroubalik commented Mar 14, 2022

RamCohen commented Mar 14, 2022

rrmcclymont commented Mar 14, 2022

JorTurFer commented Mar 14, 2022 • edited Loading

rrmcclymont commented Mar 15, 2022

RamCohen commented Mar 30, 2022

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

zroubalik commented Apr 5, 2022 • edited by github-actions bot Loading

zroubalik commented Apr 5, 2022

RamCohen commented Apr 5, 2022 • edited Loading

zroubalik left a comment

Choose a reason for hiding this comment

zroubalik left a comment

Choose a reason for hiding this comment

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

akshay201 commented Jun 8, 2023 • edited Loading

pnorth1 commented Jun 8, 2023

akshay201 commented Jun 9, 2023 • edited Loading

RamCohen commented Feb 10, 2022 •

edited

Loading

JorTurFer commented Feb 10, 2022 •

edited by github-actions bot

Loading

JorTurFer commented Mar 14, 2022 •

edited

Loading

zroubalik commented Apr 1, 2022 •

edited by github-actions bot

Loading

zroubalik commented Apr 5, 2022 •

edited by github-actions bot

Loading

RamCohen commented Apr 5, 2022 •

edited

Loading

zroubalik commented Apr 7, 2022 •

edited by github-actions bot

Loading

akshay201 commented Jun 8, 2023 •

edited

Loading

akshay201 commented Jun 9, 2023 •

edited

Loading