Skip to content

Commit 8155271

Browse files
committed
storage: replace a few panics with log.Fatal
panic causes stacks to unwind and defers to be invoked (possibly releasing locks) until eventually something either handles the panic (unlikely) or we reach the runtime and it kills the process. While the process is dying, other goroutines are still running which may encounter invalid states due to the panic. These invalid states can produce fatal errors of there own which seem impossible and are difficult to debug. For fatal errors we should use log.Fatal{,f} instead which will kill the process before any locks are released. See cockroachdb#16479
1 parent c444a60 commit 8155271

File tree

2 files changed

+12
-9
lines changed

2 files changed

+12
-9
lines changed

pkg/storage/replica.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,10 @@ func (r *Replica) assertStateLocked(ctx context.Context, reader engine.Reader) {
15801580
// TODO(dt): expose properly once #15892 is addressed.
15811581
log.Errorf(ctx, "on-disk and in-memory state diverged:\n%s", pretty.Diff(diskState, r.mu.state))
15821582
r.mu.state.Desc, diskState.Desc = nil, nil
1583-
panic(log.Safe{V: fmt.Sprintf("on-disk and in-memory state diverged: %s", pretty.Diff(diskState, r.mu.state))})
1583+
log.Fatal(ctx, log.Safe{
1584+
V: fmt.Sprintf("on-disk and in-memory state diverged: %s",
1585+
pretty.Diff(diskState, r.mu.state)),
1586+
})
15841587
}
15851588
}
15861589

@@ -4396,7 +4399,7 @@ func (r *Replica) evaluateTxnWriteBatch(
43964399
err := roachpb.NewTransactionRetryError(roachpb.RETRY_REASON_UNKNOWN)
43974400
return nil, ms, nil, EvalResult{}, roachpb.NewError(err)
43984401
}
4399-
panic("unreachable")
4402+
log.Fatal(ctx, "unreachable")
44004403
}
44014404

44024405
log.VEventf(ctx, 2, "1PC execution failed, reverting to regular execution for batch")

pkg/storage/store.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,7 @@ func NewStore(cfg StoreConfig, eng engine.Engine, nodeDesc *roachpb.NodeDescript
896896
cfg.SetDefaults()
897897

898898
if !cfg.Valid() {
899-
panic(fmt.Sprintf("invalid store configuration: %+v", &cfg))
899+
log.Fatalf(context.Background(), "invalid store configuration: %+v", &cfg)
900900
}
901901
s := &Store{
902902
cfg: cfg,
@@ -1387,7 +1387,7 @@ func (s *Store) GossipStore(ctx context.Context) error {
13871387
select {
13881388
case <-s.cfg.Gossip.Connected:
13891389
default:
1390-
panic(fmt.Sprintf("%s: not connected to gossip", s))
1390+
log.Fatalf(ctx, "not connected to gossip")
13911391
}
13921392

13931393
storeDesc, err := s.Descriptor()
@@ -2854,7 +2854,7 @@ func (s *Store) HandleRaftRequest(
28542854
) *roachpb.Error {
28552855
if len(req.Heartbeats)+len(req.HeartbeatResps) > 0 {
28562856
if req.RangeID != 0 {
2857-
panic("coalesced heartbeats must have rangeID == 0")
2857+
log.Fatalf(ctx, "coalesced heartbeats must have rangeID == 0")
28582858
}
28592859
s.uncoalesceBeats(ctx, req.Heartbeats, req.FromReplica, req.ToReplica, raftpb.MsgHeartbeat, respStream)
28602860
s.uncoalesceBeats(ctx, req.HeartbeatResps, req.FromReplica, req.ToReplica, raftpb.MsgHeartbeatResp, respStream)
@@ -2915,7 +2915,7 @@ func (s *Store) processRaftRequest(
29152915

29162916
if req.Quiesce {
29172917
if req.Message.Type != raftpb.MsgHeartbeat {
2918-
panic(fmt.Sprintf("unexpected quiesce: %+v", req))
2918+
log.Fatalf(ctx, "unexpected quiesce: %+v", req)
29192919
}
29202920
status := r.RaftStatus()
29212921
if status != nil && status.Term == req.Message.Term && status.Commit == req.Message.Commit {
@@ -3150,7 +3150,7 @@ func (s *Store) processRaftRequest(
31503150

31513151
if _, err := r.handleRaftReadyRaftMuLocked(inSnap); err != nil {
31523152
// mimic the behavior in processRaft.
3153-
panic(err)
3153+
log.Fatal(ctx, err)
31543154
}
31553155
removePlaceholder = false
31563156
return nil
@@ -3450,7 +3450,7 @@ func (s *Store) processReady(rangeID roachpb.RangeID) {
34503450
if ok {
34513451
stats, err := r.handleRaftReady(IncomingSnapshot{})
34523452
if err != nil {
3453-
panic(err) // TODO(bdarnell)
3453+
log.Fatal(r.AnnotateCtx(context.Background()), err) // TODO(bdarnell)
34543454
}
34553455
elapsed := timeutil.Since(start)
34563456
s.metrics.RaftWorkingDurationNanos.Inc(elapsed.Nanoseconds())
@@ -3579,7 +3579,7 @@ func (s *Store) sendQueuedHeartbeatsToNode(
35793579
} else if len(beats) == 0 {
35803580
msgType = raftpb.MsgHeartbeatResp
35813581
} else {
3582-
panic("cannot coalesce both heartbeats and responses")
3582+
log.Fatal(s.AnnotateCtx(context.Background()), "cannot coalesce both heartbeats and responses")
35833583
}
35843584

35853585
chReq := &RaftMessageRequest{

0 commit comments

Comments
 (0)