*: handle panic and fatal more consistently

1. etcd fatals if there is critical error in the system and operator should
do something for it
2. etcd panics if there happens something unexpected, and it should be
reported to us to debug.

etcdserver/cluster.go

@@ -164,7 +164,7 @@ func (c *Cluster) MemberByName(name string) *Member {
 	for _, m := range c.members {
 		if m.Name == name {
 			if memb != nil {
-				panic("two members with the given name exist in the cluster")
+				log.Panicf("two members with the given name %s exist", name)
 			}
 			memb = m
 		}
@@ -270,19 +270,19 @@ func (c *Cluster) SetStore(st store.Store) { c.store = st }
 func (c *Cluster) AddMember(m *Member) {
 	b, err := json.Marshal(m.RaftAttributes)
 	if err != nil {
-		log.Panicf("marshal error: %v", err)
+		log.Panicf("marshal raftAttributes should never fail: %v", err)
 	}
 	p := path.Join(memberStoreKey(m.ID), raftAttributesSuffix)
 	if _, err := c.store.Create(p, false, string(b), false, store.Permanent); err != nil {
-		log.Panicf("add raftAttributes should never fail: %v", err)
+		log.Panicf("create raftAttributes should never fail: %v", err)
 	}
 	b, err = json.Marshal(m.Attributes)
 	if err != nil {
-		log.Panicf("marshal error: %v", err)
+		log.Panicf("marshal attributes should never fail: %v", err)
 	}
 	p = path.Join(memberStoreKey(m.ID), attributesSuffix)
 	if _, err := c.store.Create(p, false, string(b), false, store.Permanent); err != nil {
-		log.Panicf("add attributes should never fail: %v", err)
+		log.Panicf("create attributes should never fail: %v", err)
 	}
 	c.members[m.ID] = m
 }
@@ -291,11 +291,11 @@ func (c *Cluster) AddMember(m *Member) {
 // The given id MUST exist, or the function panics.
 func (c *Cluster) RemoveMember(id types.ID) {
 	if _, err := c.store.Delete(memberStoreKey(id), true, true); err != nil {
-		log.Panicf("delete peer should never fail: %v", err)
+		log.Panicf("delete member should never fail: %v", err)
 	}
 	delete(c.members, id)
 	if _, err := c.store.Create(removedMemberStoreKey(id), false, "", false, store.Permanent); err != nil {
-		log.Panicf("creating RemovedMember should never fail: %v", err)
+		log.Panicf("create removedMember should never fail: %v", err)
 	}
 	c.removed[id] = true
 }

etcdserver/etcdhttp/httptypes/errors.go

@@ -18,6 +18,7 @@ package httptypes
 
 import (
 	"encoding/json"
+	"log"
 	"net/http"
 )
 
@@ -37,7 +38,7 @@ func (e HTTPError) WriteTo(w http.ResponseWriter) {
 	w.WriteHeader(e.Code)
 	b, err := json.Marshal(e)
 	if err != nil {
-		panic("unexpected json marshal error")
+		log.Panicf("marshal HTTPError should never fail: %v", err)
 	}
 	w.Write(b)
 }

etcdserver/member.go

@@ -75,7 +75,7 @@ func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.T
 // It will panic if there is no PeerURLs available in Member.
 func (m *Member) PickPeerURL() string {
 	if len(m.PeerURLs) == 0 {
-		panic("member should always have some peer url")
+		log.Panicf("member should always have some peer url")
 	}
 	return m.PeerURLs[rand.Intn(len(m.PeerURLs))]
 }

etcdserver/server.go

@@ -189,7 +189,7 @@ func NewServer(cfg *ServerConfig) (*EtcdServer, error) {
 	case !haveWAL && cfg.ClusterState == ClusterStateValueExisting:
 		cl, err := GetClusterFromPeers(cfg.Cluster.PeerURLs())
 		if err != nil {
-			log.Fatal(err)
+			return nil, fmt.Errorf("cannot fetch cluster info from peer urls: %v", err)
 		}
 		if err := cfg.Cluster.ValidateAndAssignIDs(cl.Members()); err != nil {
 			return nil, fmt.Errorf("error validating IDs from cluster %s: %v", cl, err)
@@ -316,10 +316,10 @@ func (s *EtcdServer) run() {
 			}
 
 			if err := s.storage.Save(rd.HardState, rd.Entries); err != nil {
-				log.Panicf("etcdserver: save state and entries error: %v", err)
+				log.Fatalf("etcdserver: save state and entries error: %v", err)
 			}
 			if err := s.storage.SaveSnap(rd.Snapshot); err != nil {
-				log.Panicf("etcdserver: create snapshot error: %v", err)
+				log.Fatalf("etcdserver: create snapshot error: %v", err)
 			}
 			s.send(rd.Messages)
 
@@ -338,7 +338,7 @@ func (s *EtcdServer) run() {
 			// recover from snapshot if it is more updated than current applied
 			if rd.Snapshot.Index > appliedi {
 				if err := s.store.Recovery(rd.Snapshot.Data); err != nil {
-					panic("TODO: this is bad, what do we do about it?")
+					log.Panicf("recovery store error: %v", err)
 				}
 				appliedi = rd.Snapshot.Index
 			}
@@ -371,7 +371,7 @@ func (s *EtcdServer) Stop() {
 // an error.
 func (s *EtcdServer) Do(ctx context.Context, r pb.Request) (Response, error) {
 	if r.ID == 0 {
-		panic("r.ID cannot be 0")
+		log.Panicf("request ID should never be 0")
 	}
 	if r.Method == "GET" && r.Quorum {
 		r.Method = "QGET"
@@ -484,7 +484,7 @@ func (s *EtcdServer) configure(ctx context.Context, cc raftpb.ConfChange) error
 			return err
 		}
 		if x != nil {
-			log.Panicf("unexpected return type")
+			log.Panicf("return type should always be error")
 		}
 		return nil
 	case <-ctx.Done():
@@ -571,7 +571,7 @@ func (s *EtcdServer) apply(es []raftpb.Entry, nodes []uint64) uint64 {
 			pbutil.MustUnmarshal(&cc, e.Data)
 			s.w.Trigger(cc.ID, s.applyConfChange(cc, nodes))
 		default:
-			panic("unexpected entry type")
+			log.Panicf("entry type should be either EntryNormal or EntryConfChange")
 		}
 		atomic.StoreUint64(&s.raftIndex, e.Index)
 		atomic.StoreUint64(&s.raftTerm, e.Term)
@@ -605,10 +605,10 @@ func (s *EtcdServer) applyRequest(r pb.Request) Response {
 				id := mustParseMemberIDFromKey(path.Dir(r.Path))
 				m := s.Cluster.Member(id)
 				if m == nil {
-					log.Fatalf("fetch member %s should never fail", id)
+					log.Panicf("fetch member %s should never fail", id)
 				}
 				if err := json.Unmarshal([]byte(r.Val), &m.Attributes); err != nil {
-					log.Fatalf("unmarshal %s should never fail: %v", r.Val, err)
+					log.Panicf("unmarshal %s should never fail: %v", r.Val, err)
 				}
 			}
 			return f(s.store.Set(r.Path, r.Dir, r.Val, expr))
@@ -642,10 +642,10 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, nodes []uint64) error
 	case raftpb.ConfChangeAddNode:
 		m := new(Member)
 		if err := json.Unmarshal(cc.Context, m); err != nil {
-			panic("unexpected unmarshal error")
+			log.Panicf("unmarshal member should never fail: %v", err)
 		}
 		if cc.NodeID != uint64(m.ID) {
-			panic("unexpected nodeID mismatch")
+			log.Panicf("nodeID should always be equal to member ID")
 		}
 		s.Cluster.AddMember(m)
 		log.Printf("etcdserver: added node %s to cluster", types.ID(cc.NodeID))
@@ -671,7 +671,7 @@ func (s *EtcdServer) checkConfChange(cc raftpb.ConfChange, nodes []uint64) error
 			return ErrIDNotFound
 		}
 	default:
-		panic("unexpected ConfChange type")
+		log.Panicf("ConfChange type should be either AddNode or RemoveNode")
 	}
 	return nil
 }
@@ -682,11 +682,11 @@ func (s *EtcdServer) snapshot(snapi uint64, snapnodes []uint64) {
 	// TODO: current store will never fail to do a snapshot
 	// what should we do if the store might fail?
 	if err != nil {
-		panic("TODO: this is bad, what do we do about it?")
+		log.Panicf("store save should never fail: %v", err)
 	}
 	s.node.Compact(snapi, snapnodes, d)
 	if err := s.storage.Cut(); err != nil {
-		log.Panicf("etcdserver: rotate wal file error: %v", err)
+		log.Panicf("rotate wal file should never fail: %v", err)
 	}
 }
 
@@ -729,13 +729,13 @@ func startNode(cfg *ServerConfig, ids []types.ID) (id types.ID, n raft.Node, w *
 		},
 	)
 	if w, err = wal.Create(cfg.WALDir(), metadata); err != nil {
-		log.Fatal(err)
+		log.Fatalf("etcdserver: create wal error: %v", err)
 	}
 	peers := make([]raft.Peer, len(ids))
 	for i, id := range ids {
 		ctx, err := json.Marshal((*cfg.Cluster).Member(id))
 		if err != nil {
-			log.Fatal(err)
+			log.Panicf("marshal member should never fail: %v", err)
 		}
 		peers[i] = raft.Peer{ID: uint64(id), Context: ctx}
 	}
@@ -749,11 +749,11 @@ func restartNode(cfg *ServerConfig, index uint64, snapshot *raftpb.Snapshot) (id
 	var err error
 	// restart a node from previous wal
 	if w, err = wal.OpenAtIndex(cfg.WALDir(), index); err != nil {
-		log.Fatal(err)
+		log.Fatalf("etcdserver: open wal error: %v", err)
 	}
 	wmetadata, st, ents, err := w.ReadAll()
 	if err != nil {
-		log.Fatal(err)
+		log.Fatalf("etcdserver: read wal error: %v", err)
 	}
 
 	var metadata pb.Metadata

pkg/flags/urls.go

@@ -49,7 +49,7 @@ func (us *URLsValue) String() string {
 func NewURLsValue(init string) *URLsValue {
 	v := &URLsValue{}
 	if err := v.Set(init); err != nil {
-		log.Panicf("error setting URLsValue: %v", err)
+		log.Panicf("new URLsValue should never fail: %v", err)
 	}
 	return v
 }

pkg/pbutil/pbutil.go

@@ -29,13 +29,13 @@ type Unmarshaler interface {
 func MustMarshal(m Marshaler) []byte {
 	d, err := m.Marshal()
 	if err != nil {
-		log.Panicf("pbutil: %v", err)
+		log.Panicf("marshal protobuf type should never fail: %v", err)
 	}
 	return d
 }
 
 func MustUnmarshal(um Unmarshaler, data []byte) {
 	if err := um.Unmarshal(data); err != nil {
-		log.Panicf("pbutil: %v", err)
+		log.Panicf("unmarshal protobuf type should never fail: %v", err)
 	}
 }

snap/snapshotter.go

@@ -27,6 +27,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/coreos/etcd/pkg/pbutil"
 	"github.com/coreos/etcd/raft"
 	"github.com/coreos/etcd/raft/raftpb"
 	"github.com/coreos/etcd/snap/snappb"
@@ -61,11 +62,7 @@ func (s *Snapshotter) SaveSnap(snapshot raftpb.Snapshot) error {
 
 func (s *Snapshotter) save(snapshot *raftpb.Snapshot) error {
 	fname := fmt.Sprintf("%016x-%016x%s", snapshot.Term, snapshot.Index, snapSuffix)
-	b, err := snapshot.Marshal()
-	if err != nil {
-		panic(err)
-	}
-
+	b := pbutil.MustMarshal(snapshot)
 	crc := crc32.Update(0, crcTable, b)
 	snap := snappb.Snapshot{Crc: crc, Data: b}
 	d, err := snap.Marshal()

wal/util.go

@@ -38,7 +38,7 @@ func searchIndex(names []string, index uint64) (int, bool) {
 		name := names[i]
 		_, curIndex, err := parseWalName(name)
 		if err != nil {
-			panic("parse correct name error")
+			log.Panicf("parse correct name should never fail: %v", err)
 		}
 		if index >= curIndex {
 			return i, true
@@ -54,7 +54,7 @@ func isValidSeq(names []string) bool {
 	for _, name := range names {
 		curSeq, _, err := parseWalName(name)
 		if err != nil {
-			panic("parse correct name error")
+			log.Panicf("parse correct name should never fail: %v", err)
 		}
 		if lastSeq != 0 && lastSeq != curSeq-1 {
 			return false