Skip to content

MINOR: Remove fetchQuotaMetrics and copyQuotaMetrics on close #20394

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

Merged
merged 4 commits into from
Aug 23, 2025

Conversation

jim0987795064
Copy link
Contributor

@jim0987795064 jim0987795064 commented Aug 21, 2025

  • Changes: Remove fetchQuotaMetrics and copyQuotaMetrics in RemoteLogManager on close

from: #20342 (comment)

Reviewers: Kamal Chandraprakash [email protected]

@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 Aug 21, 2025
Copy link
Contributor

@kamalcph kamalcph left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the patch!

@github-actions github-actions bot removed the triage PRs from the community label Aug 23, 2025
@kamalcph kamalcph merged commit eba9839 into apache:trunk Aug 23, 2025
23 checks passed
@kamalcph kamalcph changed the title MINOR: Remove fetchQuotaMetrics and copyQuotaMetrics MINOR: Remove fetchQuotaMetrics and copyQuotaMetrics on close Aug 23, 2025
@@ -314,6 +314,8 @@ private void removeMetrics() {
metricsGroup.removeMetric(REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT_METRIC);
metricsGroup.removeMetric(REMOTE_LOG_READER_FETCH_RATE_AND_TIME_METRIC);
remoteStorageReaderThreadPool.removeMetrics();
Utils.closeQuietly(fetchQuotaMetrics, "fetchQuotaMetrics");
Copy link
Member

Choose a reason for hiding this comment

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

@jim0987795064 Could you please add unit test for it? RemoteLogManagerTest would be a good place.

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.

3 participants