Repository navigation
Back off on-demand Metadata refreshes while a leader stays excluded (release-0.1) #97
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -377,6 +377,35 @@ func (c *WarpstreamClient) Close() { | |
| }) | ||
| } | ||
|
|
||
| // refreshBackoff paces on-demand Metadata fetches. The delay starts at floor, | ||
| // grows after each on-demand fetch up to ceiling, and returns to floor when a | ||
| // periodic fetch runs. | ||
|
koloss2001 marked this conversation as resolved.
|
||
| type refreshBackoff struct { | ||
| floor time.Duration | ||
| ceiling time.Duration | ||
| delay time.Duration | ||
| } | ||
|
|
||
| func newRefreshBackoff(floor, ceiling time.Duration) refreshBackoff { | ||
| return refreshBackoff{floor: floor, ceiling: ceiling, delay: floor} | ||
| } | ||
|
|
||
| func (b *refreshBackoff) current() time.Duration { return b.delay } | ||
|
|
||
| func (b *refreshBackoff) advance() { | ||
| // time.Duration is an int64. Doubling past MaxInt64 wraps negative. | ||
| // At or below half the ceiling, delay*2 still fits and stays within it. | ||
| if b.delay > b.ceiling/2 { | ||
| b.delay = b.ceiling | ||
| return | ||
| } | ||
| b.delay *= 2 | ||
| } | ||
|
|
||
| func (b *refreshBackoff) reset() { | ||
| b.delay = b.floor | ||
| } | ||
|
|
||
| // startBackgroundRefresh owns every post-startup AgentPool.Refresh: the | ||
| // periodic ticker and on-demand nudges from triggerRefresh. One owner means | ||
| // Refresh is never concurrent. refreshCtx (cancelled by Close) stops the loop, | ||
|
|
@@ -389,20 +418,42 @@ func (c *WarpstreamClient) startBackgroundRefresh() { | |
| defer c.refreshWG.Done() | ||
| ticker := time.NewTicker(c.cfg.MetadataRefreshInterval) | ||
| defer ticker.Stop() | ||
| backoff := newRefreshBackoff(c.cfg.OnDemandMetadataRefreshInterval, c.cfg.MetadataRefreshInterval) | ||
| for { | ||
| periodic := false | ||
| select { | ||
| case <-c.refreshCtx.Done(): | ||
| return | ||
| case <-c.refreshNowCh: | ||
| ticker.Reset(c.cfg.MetadataRefreshInterval) | ||
| startedAt := time.Now() | ||
| c.refreshPool(metadataRefreshTriggerOnDemand) | ||
| if !c.waitRefreshCooldown(time.Since(startedAt)) { | ||
| return | ||
| } | ||
| case <-ticker.C: | ||
| // A nudge queued at the same instant wins. At the ceiling the | ||
| // cooldown ends as the tick lands, and taking the tick would | ||
| // reset the delay. | ||
| select { | ||
| case <-c.refreshNowCh: | ||
| default: | ||
| periodic = true | ||
| } | ||
| } | ||
|
|
||
| if periodic { | ||
| c.refreshPool(metadataRefreshTriggerPeriodic) | ||
| // A nudge queued during this fetch must not start another after Close. | ||
| if c.refreshCtx.Err() != nil { | ||
| return | ||
| } | ||
| backoff.reset() | ||
| continue | ||
| } | ||
|
koloss2001 marked this conversation as resolved.
|
||
| ticker.Reset(c.cfg.MetadataRefreshInterval) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question, not a blocker: the backoff resets only on a periodic tick, and this line pushes the tick back on every on-demand fetch. A workload that keeps producing routing misses, such as one that produces to an unknown topic, never gets a periodic tick. It stays at the 10s ceiling indefinitely. Before this change, those misses refreshed every 1s. Is that the trade-off that you want? If so, a short note in the README "How it works" section would help, because — via Claude Code
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is intentional and follows the #95 discussion about sustained 1s all-topic Metadata refreshes creating excessive load. The first fetch can run immediately; subsequent gaps are 1s → 2s → 4s → 8s → 10s, then remain at 10s while nudges continue. Metadata still refreshes at the normal cadence. The trade-off also applies to unknown topics: the backoff is shared across the client, so persistent misses for topic A can leave a newly encountered topic B waiting for the next 10s fetch, without its own fast retry sequence. For now, I think that is acceptable. We can capture a future design for bounded resets when relevant metadata changes, I'll see if #87 has all the metrics to measure this and if not will add it there.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The README is updated |
||
| startedAt := time.Now() | ||
| c.refreshPool(metadataRefreshTriggerOnDemand) | ||
| c.waitRefreshCooldown(backoff.current(), time.Since(startedAt)) | ||
| // Return before select can take the nudge the fetch just queued. | ||
| if c.refreshCtx.Err() != nil { | ||
| return | ||
| } | ||
| backoff.advance() | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
| }() | ||
| } | ||
|
|
@@ -431,8 +482,7 @@ func (c *WarpstreamClient) refreshPool(trigger metadataRefreshTrigger) { | |
| } | ||
| c.demoter.Refresh(c.pool.Agents()) | ||
| // Nudge so a stand-in does not wait for the next periodic tick. Produce does | ||
| // not block. After a periodic tick the follow-up starts at once; later | ||
| // repeats wait out OnDemandMetadataRefreshInterval (default 1s). | ||
| // not block. The refresh loop paces the follow-up. | ||
| c.noteLeaderDrops(dropped, true) | ||
| } | ||
|
|
||
|
|
@@ -452,21 +502,18 @@ func (c *WarpstreamClient) noteLeaderDrops(dropped leaderDrops, nudge bool) { | |
| } | ||
| } | ||
|
|
||
| // waitRefreshCooldown enforces the configured minimum between on-demand | ||
| // refresh start times. Time spent fetching already counts toward the interval. | ||
| // Returns false during close so the refresh loop exits without spinning. | ||
| func (c *WarpstreamClient) waitRefreshCooldown(elapsed time.Duration) bool { | ||
| remaining := c.cfg.OnDemandMetadataRefreshInterval - elapsed | ||
| // waitRefreshCooldown sleeps out the rest of delay. Time already spent | ||
| // fetching counts. Close cancels refreshCtx and the wait returns. | ||
| func (c *WarpstreamClient) waitRefreshCooldown(delay, elapsed time.Duration) { | ||
| remaining := delay - elapsed | ||
| if remaining <= 0 { | ||
| return c.refreshCtx.Err() == nil | ||
| return | ||
| } | ||
| t := time.NewTimer(remaining) | ||
| defer t.Stop() | ||
| select { | ||
| case <-c.refreshCtx.Done(): | ||
| return false | ||
| case <-t.C: | ||
| return true | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.