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

refactor: reuse timers instead of time.After in loops #687

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion bitswap/network/ipfs_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,13 @@ func (s *streamMessageSender) multiAttempt(ctx context.Context, fn func() error)
return err
}

timer := time.NewTimer(s.opts.SendErrorBackoff)
defer timer.Stop()

select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(s.opts.SendErrorBackoff):
case <-timer.C:
// wait a short time in case disconnect notifications are still propagating
log.Infof("send message to %s failed but context was not Done: %s", s.to, err)
}
Expand Down
6 changes: 5 additions & 1 deletion bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,19 @@
ctx, cancel := context.WithCancel(ctx)
defer cancel()
go func() {
timer := time.NewTimer(time.Second)
defer timer.Stop()

Check warning on line 309 in bootstrap/bootstrap.go

View check run for this annotation

Codecov / codecov/patch

bootstrap/bootstrap.go#L307-L309

Added lines #L307 - L309 were not covered by tests
for {
select {
case <-ctx.Done():
return
case <-time.After(1 * time.Second):
case <-timer.C:

Check warning on line 314 in bootstrap/bootstrap.go

View check run for this annotation

Codecov / codecov/patch

bootstrap/bootstrap.go#L314

Added line #L314 was not covered by tests
if int(atomic.LoadUint64(&connected)) >= needed {
cancel()
return
}
timer.Reset(time.Second)

Check warning on line 319 in bootstrap/bootstrap.go

View check run for this annotation

Codecov / codecov/patch

bootstrap/bootstrap.go#L319

Added line #L319 was not covered by tests
}
}
}()
Expand Down
11 changes: 10 additions & 1 deletion mfs/repub.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,27 @@

// 2. If we have a value to publish, publish it now.
if toPublish.Defined() {
var timer *time.Timer
for {
err := rp.pubfunc(rp.ctx, toPublish)
if err == nil {
break
}

if timer == nil {
timer = time.NewTimer(rp.RetryTimeout)
defer timer.Stop()
} else {
timer.Reset(rp.RetryTimeout)
}

Check warning on line 184 in mfs/repub.go

View check run for this annotation

Codecov / codecov/patch

mfs/repub.go#L179-L184

Added lines #L179 - L184 were not covered by tests

// Keep retrying until we succeed or we abort.
// TODO(steb): We could try pulling new values
// off `update` but that's not critical (and
// complicates this code a bit). We'll pull off
// a new value on the next loop through.
select {
case <-time.After(rp.RetryTimeout):
case <-timer.C:

Check warning on line 192 in mfs/repub.go

View check run for this annotation

Codecov / codecov/patch

mfs/repub.go#L192

Added line #L192 was not covered by tests
case <-rp.ctx.Done():
return
}
Expand Down