Skip to content

Commit

Permalink
kvserver: plug a tracing span leak
Browse files Browse the repository at this point in the history
Fixes #60677, removing a stop-gap introduced in #59992.

We were previously leaking  "async consensus" spans, which was possible
when a given proposal was never flushed out of the replica's proposal
buffer. On server shut down, this buffered proposal was never finished,
and thus the embedded span never closed. We now add a closer to clean up
after ourselves.

Release justification: bug fixes and low-risk updates to new
functionality.

Release note: None
  • Loading branch information
irfansharif committed Mar 1, 2021
1 parent 65ce294 commit 0c032ac
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
7 changes: 0 additions & 7 deletions pkg/kv/kvserver/replica_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,6 @@ func (r *Replica) evalAndPropose(
// Fork the proposal's context span so that the proposal's context
// can outlive the original proposer's context.
proposal.ctx, proposal.sp = tracing.ForkSpan(ctx, "async consensus")
{
// This span sometimes leaks. Disable it for the time being.
//
// Tracked in: https://github.com/cockroachdb/cockroach/issues/60677
proposal.sp.Finish()
proposal.sp = nil
}

// Signal the proposal's response channel immediately.
reply := *proposal.Local.Reply
Expand Down
20 changes: 20 additions & 0 deletions pkg/kv/kvserver/store_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,26 @@ func (s *Store) processRaft(ctx context.Context) {
s.stopper.AddCloser(stop.CloserFn(func() {
s.cfg.Transport.Stop(s.StoreID())
}))

// We'll want to cancel all in-flight proposals. Proposals embed tracing
// spans in them, and we don't want to be leaking any.
s.stopper.AddCloser(stop.CloserFn(func() {
s.VisitReplicas(func(r *Replica) (more bool) {
r.mu.proposalBuf.FlushLockedWithoutProposing(ctx)
r.mu.Lock()
for k, prop := range r.mu.proposals {
delete(r.mu.proposals, k)
prop.finishApplication(
context.Background(),
proposalResult{
Err: roachpb.NewError(roachpb.NewAmbiguousResultError("store is stopping")),
},
)
}
r.mu.Unlock()
return true
})
}))
}

func (s *Store) raftTickLoop(ctx context.Context) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutils/testcluster/testcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (tc *TestCluster) stopServers(ctx context.Context) {
return nil
}
var buf strings.Builder
buf.WriteString("unexpectedly found active spans:\n")
buf.WriteString(fmt.Sprintf("unexpectedly found %d active spans:\n", len(sps)))
for _, sp := range sps {
fmt.Fprintln(&buf, sp.GetRecording())
fmt.Fprintln(&buf)
Expand Down

0 comments on commit 0c032ac

Please sign in to comment.