Skip to content

[ENHANCEMENT] ring/backoff: reuse timers in lifecycler and backoff loops#7270

Open
sandy2008 wants to merge 3 commits intocortexproject:masterfrom
sandy2008:master
Open

[ENHANCEMENT] ring/backoff: reuse timers in lifecycler and backoff loops#7270
sandy2008 wants to merge 3 commits intocortexproject:masterfrom
sandy2008:master

Conversation

@sandy2008
Copy link
Contributor

@sandy2008 sandy2008 commented Feb 17, 2026

What this PR does:

  • Replaces repeated time.After(...) calls with reusable time.Timer instances in:
    • pkg/ring/lifecycler.go
    • pkg/ring/basic_lifecycler.go
    • pkg/util/backoff/backoff.go
  • Adds shared safe timer utilities (stopAndDrainTimer, resetTimer) in pkg/ring/ticker.go to avoid timer channel races/leaks.
  • Optimizes DynamoDB CAS by preallocating putRequests map capacity in pkg/ring/kv/dynamodb/client.go.
  • Keeps behavior unchanged while reducing allocations in hot loops.

Performance validation (local -benchmem):

  • Watch loop (time.After vs reusable timer): 248 B/op, 3 allocs/op -> 0 B/op, 0 allocs/op.
  • Backoff wait (time.After style vs reusable timer): 248 B/op, 3 allocs/op -> 0 B/op, 0 allocs/op.
  • Observed small ns/op improvement in both benchmarks.

Which issue(s) this PR fixes:
Similar to #7266

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com>
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

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

I am concerned also that unit tests on backoff.go don't cover these changes.
could you add them?

@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 18, 2026
Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com>
@sandy2008
Copy link
Contributor Author

@friedrichg Oops.. forgot to modernize, let me fix.

Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com>
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments