Skip to content

feat(collector): clean up expired tables during hotspot detection#2369

Merged
empiredan merged 3 commits intoapache:masterfrom
empiredan:go-collector-hotspot-retire-expired-tables
Mar 20, 2026
Merged

feat(collector): clean up expired tables during hotspot detection#2369
empiredan merged 3 commits intoapache:masterfrom
empiredan:go-collector-hotspot-retire-expired-tables

Conversation

@empiredan
Copy link
Copy Markdown
Contributor

@empiredan empiredan commented Feb 12, 2026

#2358

Currently, when collecting hotspot-related metrics, the Go collector only adds
new tables. If a large number of tables are deleted, the collector may still retain
those deleted tables in memory, leading to excessive memory usage.

This PR introduces a table cleanup mechanism: if no metric data has been collected
for a table over a prolonged period, the table will be removed to release the memory
it occupies.

@empiredan empiredan marked this pull request as ready for review February 13, 2026 04:20
Copy link
Copy Markdown

@acelyc111-bot acelyc111-bot left a comment

Choose a reason for hiding this comment

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

Review: Clean up expired tables during hotspot detection

Summary: Adds periodic cleanup of expired table analyzers in the hotspot partition detector, controlled by a new retention_period config (default 24h). Also fixes a TODO by using proper HostPort.Equal() comparison.

What's good:

  • Clean implementation with proper goroutine lifecycle management (context + WaitGroup for graceful shutdown)
  • retireExpiredTables uses a single lock acquisition per cleanup cycle — efficient
  • Expiration timestamp is set at analysis time (not creation time), so active tables never expire
  • Bonus: Fixed the TODO(wangdan) for HostPort.Equal() usage
  • Config validation ensures RetentionPeriod > 0

Minor notes:

  • The checkExpiration ticker interval equals RetentionPeriod (24h default) — this means tables could live up to 2x retention period in the worst case. Consider using a shorter check interval (e.g., RetentionPeriod / 4 or a fixed 1h) for tighter cleanup. Not a blocker though.
  • Config file has some formatting cleanup (extra spaces removed) — fine, but ideally in a separate commit

Verdict: ✅ Approve — Well-structured, properly tested. Ready to merge.

@empiredan empiredan merged commit 3ab03aa into apache:master Mar 20, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants