Skip to content

Commit

Permalink
Merge #45460
Browse files Browse the repository at this point in the history
45460: closedts: avoid logging failures on startup r=tbg a=ajwerner

Before this commit it was commonly the case that nodes would fail to move the
closed timestamp due to not being live when they start up. This warning was
concerning to customers. Now we'll track whether we've ever been live and only
log if we find that we are not live later.

Fixes #44846

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
craig[bot] and ajwerner committed Feb 27, 2020
2 parents 3170612 + 99021d6 commit eae5a92
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions pkg/storage/closedts/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ func (p *Provider) runCloser(ctx context.Context) {
}
}
closedts.TargetDuration.SetOnChange(&p.cfg.Settings.SV, confChanged)

// Track whether we've ever been live to avoid logging warnings about not
// being live during node startup.
var everBeenLive bool
var t timeutil.Timer
defer t.Stop()
for {
Expand All @@ -146,13 +148,14 @@ func (p *Provider) runCloser(ctx context.Context) {
next, liveAtEpoch, err := p.cfg.Clock(p.cfg.NodeID)
next.WallTime -= int64(targetDuration)
if err != nil {
if p.everyClockLog.ShouldLog() {
if everBeenLive && p.everyClockLog.ShouldLog() {
log.Warningf(ctx, "unable to move closed timestamp forward: %+v", err)
}
// Broadcast even if nothing new was queued, so that the subscribers
// loop to check their client's context.
p.mu.Broadcast()
} else {
everBeenLive = true
// Close may fail if the data being closed does not correspond to the
// current liveAtEpoch.
closed, m, ok := p.cfg.Close(next, liveAtEpoch)
Expand Down

0 comments on commit eae5a92

Please sign in to comment.