-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Introduce new scaling logic with fix orphan pod issue #1214
Conversation
Signed-off-by: Tsuyoshi Ushio <[email protected]>
Signed-off-by: Tsuyoshi Ushio <[email protected]>
5b71519
to
6b5c0a4
Compare
Signed-off-by: Tsuyoshi Ushio <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!
Thanks for fix! When it will be in Helm chart 2.0-RC? |
* Introduce new scaling logic with fix orphan pod issue Signed-off-by: Tsuyoshi Ushio <[email protected]> * update yamls Signed-off-by: Tsuyoshi Ushio <[email protected]> * Remove to fit the coding style Signed-off-by: Tsuyoshi Ushio <[email protected]>
@TsuyoshiUshio Actually, i tried all scaling profiles, and still can't get to achieve that when a long executing job is running and a new RabbitMQ is received a new job get created
|
I introduce a change for the scaling logic for scaled job.
It is going to solve these issues. I'd like to share this PR for reviewing it first.
Old Logic
The number of newly created jobs are
queueLength - runningCount
.New Logic
The number of newly created jobs are
Limitation
ServiceBusScaler using
*queueEntity.CountDetails.ActiveMessageCount
to fetch the ActiveMessageCount, However, this is not right as a queue length. The value includes the message that is locked. That means, If you receive a queue, and not complete the message, it is locked, and other client can't consume it. However,ActiveMessageCount
includes the locked message. I tried other way to fetch theActiveMessageCount - LockedMessageCount
, however, I couldn't find the way to do it until now.What I did
Checklist
Fixes #
#1207 (comment)
#1186
#1211