Skip to content
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

KAFKA-18597 Fix max-buffer-utilization-percent is always 0 #18627

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

LoganZhuZzz
Copy link
Contributor

@LoganZhuZzz LoganZhuZzz commented Jan 19, 2025

https://issues.apache.org/jira/browse/KAFKA-18597

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community core Kafka Broker small Small PRs labels Jan 19, 2025
@chia7712
Copy link
Member

@LoganZhuZzz This is a bug fix, so could you please add unit test?

@LoganZhuZzz
Copy link
Contributor Author

@LoganZhuZzz This is a bug fix, so could you please add unit test?

I’ll add the unit test later.

@chia7712 chia7712 changed the title KAFKA-18597: Fix max-buffer-utilization-percent is always 0 KAFKA-18597 Fix max-buffer-utilization-percent is always 0 Jan 19, 2025
@chia7712
Copy link
Member

@LoganZhuZzz additionally, please take a look at @ijuma's comment (#17390 (comment))

@github-actions github-actions bot removed the small Small PRs label Jan 20, 2025
@github-actions github-actions bot added the small Small PRs label Jan 20, 2025
@github-actions github-actions bot removed the small Small PRs label Jan 20, 2025
@github-actions github-actions bot added the small Small PRs label Jan 20, 2025
@LoganZhuZzz
Copy link
Contributor Author

@LoganZhuZzz additionally, please take a look at @ijuma's comment (#17390 (comment))

@chia7712 PTAL

Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@LoganZhuZzz thanks for this patch!


/* a metric to track the maximum utilization of any thread's buffer in the last cleaning */
metricsGroup.newGauge(MaxBufferUtilizationPercentMetricName,
() => maxOverCleanerThreads(_.lastStats.bufferUtilization) * 100)
() => (maxOverCleanerThreads(_.lastStats.bufferUtilization) * 100).toInt)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add unit test for other impacted metrics?

}

@Test
def testMaxBufferUtilizationPercentMetric(): Unit = {
Copy link
Member

Choose a reason for hiding this comment

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

It seems this new unit test is similar to testMaxOverCleanerThreads, right? If so, could you please remove testMaxOverCleanerThreads?

@github-actions github-actions bot removed the triage PRs from the community label Jan 24, 2025
…anerTest

- Added testMaxBufferUtilizationPercentMetric to validate maximum buffer utilization metric.
- Added testMaxCleanTimeMetric to verify the maximum clean time metric.
- Added testMaxCompactionDelayMetrics to ensure correctness of maximum compaction delay metric.
@github-actions github-actions bot removed the small Small PRs label Jan 24, 2025
@chia7712 chia7712 merged commit 356f0d8 into apache:trunk Jan 24, 2025
8 of 9 checks passed
pranavt84 pushed a commit to pranavt84/kafka that referenced this pull request Jan 27, 2025
airlock-confluentinc bot pushed a commit to confluentinc/kafka that referenced this pull request Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants