Skip to content

Cost-based autoscaler: allow scaling action only when metrics are available#19028

Merged
kfaraz merged 6 commits intoapache:masterfrom
Fly-Style:cba-remove-processing-rate
Feb 19, 2026
Merged

Cost-based autoscaler: allow scaling action only when metrics are available#19028
kfaraz merged 6 commits intoapache:masterfrom
Fly-Style:cba-remove-processing-rate

Conversation

@Fly-Style
Copy link
Contributor

@Fly-Style Fly-Style commented Feb 18, 2026

Default processing rate was a crutch that allowed autoscaler to start scaling actions earlier than the first metrics became available. In fact, it is confusing parameter and actually a crutch, which was removed in that patch.

This PR has:

  • been self-reviewed.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@Fly-Style Fly-Style changed the title Remove default proc. rate; allow scaling only when metrics are available Cost-based autoscaler: allow scaling action only when metrics are available Feb 18, 2026
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, the test should verify that scaling action is skipped when moving average rate is not available. The test should care less about the internal implementation details of the target and more about its overall behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was added mostly to satisfy jacoco test coverage :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😛 yay, jacoco!

Let's take the opportunity to make it a useful test then. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion to improve test behaviour.

public int computeTaskCountForScaleAction()
{
lastKnownMetrics = collectMetrics();
if (lastKnownMetrics == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@Fly-Style Fly-Style requested a review from kfaraz February 18, 2026 14:18
@Fly-Style Fly-Style force-pushed the cba-remove-processing-rate branch from 44286a6 to c69f335 Compare February 18, 2026 14:47
@kfaraz kfaraz merged commit 68785ba into apache:master Feb 19, 2026
48 of 72 checks passed
@kfaraz
Copy link
Contributor

kfaraz commented Feb 19, 2026

Merged the PR since the failure in the docker tests is unrelated and has been seen on other PRs.
Attempt to fix the docker tests here #18849

@Fly-Style Fly-Style deleted the cba-remove-processing-rate branch February 19, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments