Skip to content

fix(metric): remove redundant timer cleanup code#7675

Merged
limingxinleo merged 2 commits intohyperf:masterfrom
huangdijia:feature/metric-timer-cleanup
Jan 25, 2026
Merged

fix(metric): remove redundant timer cleanup code#7675
limingxinleo merged 2 commits intohyperf:masterfrom
huangdijia:feature/metric-timer-cleanup

Conversation

@huangdijia
Copy link
Member

Summary

Remove redundant timer cleanup code from metric listeners since the Timer component now handles cleanup automatically on worker exit.

Changes

  • Remove timerId variable assignment and cleanup coroutines
  • Remove unused imports: CoordinatorManager, Constants, and Coroutine
  • Simplify timer usage across all metric listeners

Files Modified

  • src/metric/src/Listener/MetricBufferWatcher.php
  • src/metric/src/Listener/OnBeforeHandle.php
  • src/metric/src/Listener/OnCoroutineServerStart.php
  • src/metric/src/Listener/OnMetricFactoryReady.php
  • src/metric/src/Listener/OnWorkerStart.php
  • src/metric/src/Listener/PoolWatcher.php
  • src/metric/src/Listener/QueueWatcher.php

Test Plan

  • Verify metric collection still works correctly
  • Test timer cleanup behavior on worker exit
  • Confirm no memory leaks from timer cleanup changes
  • Run existing metric-related test suites

Breaking Changes

No breaking changes - this is an internal cleanup that removes redundant code.

The Timer component now handles cleanup automatically on worker exit,
so the manual timer cleanup logic in metric listeners is no longer needed.
This removes the CoordinatorManager and Coroutine dependencies that were
only used for timer cleanup.

Changes:
- Remove timerId variable assignment and cleanup coroutines
- Remove unused imports: CoordinatorManager, Constants, and Coroutine
- Simplify timer usage across all metric listeners

Affected files:
- MetricBufferWatcher
- OnBeforeHandle
- OnCoroutineServerStart
- OnMetricFactoryReady
- OnWorkerStart
- PoolWatcher
- QueueWatcher
Copilot AI review requested due to automatic review settings December 18, 2025 09:45
Copy link

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 removes redundant timer cleanup code from metric listeners since the Timer component now handles cleanup automatically when workers exit via the coordinator pattern.

  • Removes manual timer ID tracking and cleanup coroutines across all metric listeners
  • Removes unused imports: CoordinatorManager, Constants, and Coroutine
  • Simplifies timer usage by eliminating the need for explicit cleanup on worker exit

Reviewed changes

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

Show a summary per file
File Description
src/metric/src/Listener/QueueWatcher.php Removes timer cleanup coroutine and unused imports for queue metrics monitoring
src/metric/src/Listener/PoolWatcher.php Removes timer cleanup coroutine and unused imports for connection pool monitoring
src/metric/src/Listener/OnWorkerStart.php Removes timer cleanup coroutine and coordinator imports for worker-level metrics
src/metric/src/Listener/OnMetricFactoryReady.php Removes timer cleanup coroutine and coordinator imports for metric factory readiness
src/metric/src/Listener/OnCoroutineServerStart.php Removes timer cleanup coroutine and coordinator imports for coroutine server metrics
src/metric/src/Listener/OnBeforeHandle.php Removes timer cleanup coroutine and coordinator imports for command metric collection
src/metric/src/Listener/MetricBufferWatcher.php Removes timer cleanup coroutine and unused imports for metric buffer flushing

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

@limingxinleo limingxinleo merged commit 378283e into hyperf:master Jan 25, 2026
77 checks passed
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