Skip to content
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

chore: Clean up old streams periodically in RF-1 ingester #13511

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

benclive
Copy link
Contributor

@benclive benclive commented Jul 12, 2024

What this PR does / why we need it:

  • RF-1 ingester doesn't expire chunks or have a convenient clean up point for a stream such as after flushing a chunk
  • We've observed a memory leak here because it will keep stream metadata around indefinitely once seen. This is made slightly more obvious because there are fewer ingesters so they handle more streams each.
  • This adds a periodic flush to remove any stream metadata older than 5 minutes (configurable) which should keep memory usage in line.
  • I also took the opportunity to clean up a few more "chunk" related flags.

Fixes https://github.com/grafana/loki-private/issues/1034

@benclive benclive requested a review from a team as a code owner July 12, 2024 15:12
for {
select {
case <-flushTicker.C:
i.doFlushTick()
case <-streamRetentionTicker.C:
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to move this ticker check to a separate go routine loop so that doFlushTick would never be blocked by the stream cleanup? I imagine flushing data is more important than clearing memory at least in the short term

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, thats a good point. I'll create a separate goroutine

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 because I want to remove this select :)

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Jul 15, 2024
pkg/ingester-rf1/ingester.go Outdated Show resolved Hide resolved
pkg/ingester-rf1/ingester.go Outdated Show resolved Hide resolved
@benclive benclive force-pushed the benclive/prune-old-streams-rf1 branch from ed773c9 to 667ff4c Compare July 16, 2024 17:10
@benclive benclive force-pushed the benclive/prune-old-streams-rf1 branch from 667ff4c to 34a10fb Compare July 17, 2024 08:56
@benclive benclive merged commit c0f5fb6 into main Jul 17, 2024
62 checks passed
@benclive benclive deleted the benclive/prune-old-streams-rf1 branch July 17, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants