Skip to content

KAFKA-20368: Use LinkedHashMap for deterministic metric tags order in RLMExpirationTask#21931

Open
cychiu8 wants to merge 2 commits intoapache:trunkfrom
cychiu8:20368-linkedHashMap-for-deterministic-metric-tags-order
Open

KAFKA-20368: Use LinkedHashMap for deterministic metric tags order in RLMExpirationTask#21931
cychiu8 wants to merge 2 commits intoapache:trunkfrom
cychiu8:20368-linkedHashMap-for-deterministic-metric-tags-order

Conversation

@cychiu8
Copy link
Copy Markdown
Contributor

@cychiu8 cychiu8 commented Apr 1, 2026

Summary: use MetricsUtils.getTags to handle metricTags in RLMExpirationTask.

Copilot AI review requested due to automatic review settings April 1, 2026 23:00
@github-actions github-actions bot added triage PRs from the community storage Pull requests that target the storage module tiered-storage Related to the Tiered Storage feature small Small PRs labels Apr 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 makes RemoteLogManager’s per-partition retention metrics produce deterministic JMX MBean tag ordering by constructing the tag map via MetricsUtils.getTags (which preserves insertion order), and updates tests to match the new stable tag order and validate metric cleanup on task cancellation.

Changes:

  • Switch RLMExpirationTask metric tag creation to MetricsUtils.getTags(...) for deterministic tag iteration order.
  • Update JMX metric name expectations in RemoteLogManagerTest to match the stable topic,partition order.
  • Extend the task-cancellation metrics test to assert the metrics are actually deregistered from the Yammer registry.

Reviewed changes

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

File Description
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManager.java Uses MetricsUtils.getTags to build ordered metricTags for retention gauges in RLMExpirationTask.
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerTest.java Adjusts expected JMX suffix ordering for retention metrics and adds assertions that cancellation deregisters them.

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

@cychiu8
Copy link
Copy Markdown
Contributor Author

cychiu8 commented Apr 1, 2026

the JMX metric of kafka.log.remote:type=RemoteLogManager
before change: (partition → topic)
image

after change: (topic → partition)
image

Copy link
Copy Markdown
Contributor

@nileshkumar3 nileshkumar3 left a comment

Choose a reason for hiding this comment

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

LGTM. Using deterministic tag ordering here makes sense, and the JMX metric name assertions are updated consistently with the new tag order.

@github-actions github-actions bot removed the triage PRs from the community label Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved small Small PRs storage Pull requests that target the storage module tiered-storage Related to the Tiered Storage feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants