Skip to content

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

Merged
friedrichg merged 3 commits intocortexproject:masterfrom
sandy2008:master
Feb 20, 2026
Merged

[ENHANCEMENT] ring/backoff: reuse timers in lifecycler and backoff loops#7270
friedrichg merged 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
@sandy2008
Copy link
Contributor Author

LGTM. Thank you!

@friedrichg Thank you :) Hope can get merged soon.

Copy link
Member

@SungJin1212 SungJin1212 left a comment

Choose a reason for hiding this comment

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

Nice improvement, LGTM

@friedrichg friedrichg merged commit dcc1772 into cortexproject:master Feb 20, 2026
61 of 64 checks passed
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.

3 participants

Comments