Skip to content

fix(inkless:metrics): exclude consolidating partitions from URP metrics#640

Open
jeqo wants to merge 2 commits into
mainfrom
jeqo/ts-consolidation-not-urp
Open

fix(inkless:metrics): exclude consolidating partitions from URP metrics#640
jeqo wants to merge 2 commits into
mainfrom
jeqo/ts-consolidation-not-urp

Conversation

@jeqo

@jeqo jeqo commented Jun 12, 2026

Copy link
Copy Markdown
Contributor
  • Exclude consolidating diskless partitions from underReplicatedPartitionCount, atMinIsrPartitionCount, and underMinIsrPartitionCount gauges
  • These partitions never materialize followers (ReplicaManager skips getOrCreatePartition for them), so they always trip URP-related alerts despite being healthy from a consolidation standpoint
  • Consolidation health is already observable via dedicated ConsolidationLocalLag/TotalLag metrics

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates broker-level under-replicated partition (URP) metrics to exclude consolidating diskless topics, which can appear perpetually under-replicated because their follower replicas never materialize, and adds a unit test to lock in the intended behavior.

Changes:

  • Exclude consolidating diskless topics from ReplicaManager.underReplicatedPartitionCount.
  • Add a unit test asserting URP count ignores consolidating diskless partitions but still counts classic under-replication.
  • Update .gitignore to ignore Inkless sync output and core/data/.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
core/src/main/scala/kafka/server/ReplicaManager.scala Adjusts URP counting logic to skip consolidating diskless topics.
core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala Adds a regression test for the updated URP counting behavior.
.gitignore Ignores additional generated/local data directories.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala
Comment thread core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala Outdated
@jeqo jeqo force-pushed the jeqo/ts-consolidation-not-urp branch from 3552e7f to 1bf3920 Compare June 12, 2026 08:10
@jeqo jeqo requested a review from Copilot June 12, 2026 08:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala
@jeqo jeqo force-pushed the jeqo/ts-consolidation-not-urp branch from 1bf3920 to 58132ec Compare June 12, 2026 08:21
@jeqo jeqo requested a review from Copilot June 12, 2026 08:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.

@jeqo jeqo marked this pull request as ready for review June 12, 2026 08:25
@jeqo jeqo marked this pull request as draft June 12, 2026 09:02
@jeqo

jeqo commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Drafting as I see under-min-isr as well Done

@jeqo jeqo force-pushed the jeqo/ts-consolidation-not-urp branch from 58132ec to b6c8942 Compare June 12, 2026 09:46
@jeqo jeqo marked this pull request as ready for review June 12, 2026 09:49
Consolidating diskless partitions never materialize followers
(ReplicaManager skips getOrCreatePartition for them), so they
always trip URP-related gauges (underReplicatedPartitionCount,
atMinIsrPartitionCount, underMinIsrPartitionCount) despite having
dedicated ConsolidationLocalLag/TotalLag metrics for observability.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jeqo jeqo force-pushed the jeqo/ts-consolidation-not-urp branch from b6c8942 to 17e48b3 Compare June 12, 2026 09:53
@jeqo jeqo changed the title fix(inkless:metrics): exclude consolidating partitions from URP count fix(inkless:metrics): exclude consolidating partitions from URP metrics Jun 12, 2026
@jeqo jeqo requested a review from Copilot June 12, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants