Skip to content

Commit

Permalink
sqlliveness: ensure that the heartbeat doesn't get canceled
Browse files Browse the repository at this point in the history
Before this patch, the async heartbeat loop was respecting the
cancelation of the context passed to Instance.Start(). That's a bad
idea, since the async task should be detached from the caller.

Release note: None
  • Loading branch information
andreimatei committed Oct 31, 2022
1 parent ae0ae72 commit f135bf0
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
1 change: 1 addition & 0 deletions pkg/sql/sqlliveness/slinstance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"//pkg/util/uuid",
"@com_github_cockroachdb_logtags//:logtags",
],
)

Expand Down
9 changes: 7 additions & 2 deletions pkg/sql/sqlliveness/slinstance/slinstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/cockroachdb/logtags"
)

var (
Expand Down Expand Up @@ -240,6 +241,8 @@ func (l *Instance) heartbeatLoop(ctx context.Context) {
defer func() {
log.Warning(ctx, "exiting heartbeat loop")
}()
// Operations below retry endlessly after the stopper started quiescing if we
// don't cancel their ctx.
ctx, cancel := l.stopper.WithCancelOnQuiesce(ctx)
defer cancel()
t := timeutil.NewTimer()
Expand Down Expand Up @@ -334,9 +337,11 @@ func (l *Instance) Start(ctx context.Context) {
if l.mu.started {
return
}
log.Infof(ctx, "starting SQL liveness instance")
_ = l.stopper.RunAsyncTask(ctx, "slinstance", l.heartbeatLoop)
l.mu.started = true
log.Infof(ctx, "starting SQL liveness instance")
// Detach from ctx's cancelation.
taskCtx := logtags.WithTags(context.Background(), logtags.FromContext(ctx))
_ = l.stopper.RunAsyncTask(taskCtx, "slinstance", l.heartbeatLoop)
}

// Session returns a live session id. For each Sqlliveness instance the
Expand Down

0 comments on commit f135bf0

Please sign in to comment.