etcdserver: avoid deadlock caused by adding members with wrong peer URLs
Current membership changing functionality of etcd seems to have a problem which can cause deadlock. How to produce: 1. construct N node cluster 2. add N new nodes with etcdctl member add, without starting the new members What happens: After finishing add N nodes, a total number of the cluster becomes 2 * N and a quorum number of the cluster becomes N + 1. It means membership change requires at least N + 1 nodes because Raft treats membership information in its log like other ordinal log append requests. Assume the peer URLs of the added nodes are wrong because of miss operation or bugs in wrapping program which launch etcd. In such a case, both of adding and removing members are impossible because the quorum isn't preserved. Of course ordinal requests cannot be served. The cluster would seem to be deadlock. Of course, the best practice of adding new nodes is adding one node and let the node start one by one. However, the effect of this problem is so serious. I think preventing the problem forcibly would be valuable. Solution: This patch lets etcd forbid adding a new node if the operation changes quorum and the number of changed quorum is larger than a number of running nodes. If etcd is launched with a newly added option -strict-reconfig-check, the checking logic is activated. If the option isn't passed, default behavior of reconfig is kept. Fixes https://github.com/coreos/etcd/issues/3477release-2.3
parent
28a371471a
commit
6974fc63ed
|
@ -95,6 +95,7 @@ type config struct {
|
|||
fallback *flags.StringsFlag
|
||||
initialCluster string
|
||||
initialClusterToken string
|
||||
strictReconfigCheck bool
|
||||
|
||||
// proxy
|
||||
proxy *flags.StringsFlag
|
||||
|
@ -177,6 +178,7 @@ func NewConfig() *config {
|
|||
// Should never happen.
|
||||
plog.Panicf("unexpected error setting up clusterStateFlag: %v", err)
|
||||
}
|
||||
fs.BoolVar(&cfg.strictReconfigCheck, "strict-reconfig-check", false, "Reject reconfiguration that might cause quorum loss")
|
||||
|
||||
// proxy
|
||||
fs.Var(cfg.proxy, "proxy", fmt.Sprintf("Valid values include %s", strings.Join(cfg.proxy.Values, ", ")))
|
||||
|
|
|
@ -265,6 +265,7 @@ func startEtcd(cfg *config) (<-chan struct{}, error) {
|
|||
TickMs: cfg.TickMs,
|
||||
ElectionTicks: cfg.electionTicks(),
|
||||
V3demo: cfg.v3demo,
|
||||
StrictReconfigCheck: cfg.strictReconfigCheck,
|
||||
}
|
||||
var s *etcdserver.EtcdServer
|
||||
s, err = etcdserver.NewServer(srvcfg)
|
||||
|
|
|
@ -368,6 +368,34 @@ func (c *cluster) SetVersion(ver *semver.Version) {
|
|||
MustDetectDowngrade(c.version)
|
||||
}
|
||||
|
||||
func (c *cluster) isReadyToAddNewMember() bool {
|
||||
nmembers := 1
|
||||
nstarted := 0
|
||||
|
||||
for _, member := range c.members {
|
||||
if member.IsStarted() {
|
||||
nstarted++
|
||||
}
|
||||
nmembers++
|
||||
}
|
||||
|
||||
if nstarted == 1 {
|
||||
// a case of adding a new node to 1-member cluster for restoring cluster data
|
||||
// https://github.com/coreos/etcd/blob/master/Documentation/admin_guide.md#restoring-the-cluster
|
||||
|
||||
plog.Debugf("The number of started member is 1. This cluster can accept add member request.")
|
||||
return true
|
||||
}
|
||||
|
||||
nquorum := nmembers/2 + 1
|
||||
if nstarted < nquorum {
|
||||
plog.Warningf("Reject add member request: the number of started member (%d) will be less than the quorum number of the cluster (%d)", nstarted, nquorum)
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func membersFromStore(st store.Store) (map[types.ID]*Member, map[types.ID]bool) {
|
||||
members := make(map[types.ID]*Member)
|
||||
removed := make(map[types.ID]bool)
|
||||
|
|
|
@ -49,6 +49,8 @@ type ServerConfig struct {
|
|||
ElectionTicks int
|
||||
|
||||
V3demo bool
|
||||
|
||||
StrictReconfigCheck bool
|
||||
}
|
||||
|
||||
// VerifyBootstrapConfig sanity-checks the initial config for bootstrap case
|
||||
|
|
|
@ -31,6 +31,7 @@ var (
|
|||
ErrTimeout = errors.New("etcdserver: request timed out")
|
||||
ErrTimeoutDueToLeaderFail = errors.New("etcdserver: request timed out, possibly due to previous leader failure")
|
||||
ErrTimeoutDueToConnectionLost = errors.New("etcdserver: request timed out, possibly due to connection lost")
|
||||
ErrNotEnoughStartedMembers = errors.New("etcdserver: re-configuration failed due to not enough started members")
|
||||
)
|
||||
|
||||
func isKeyNotFound(err error) bool {
|
||||
|
|
|
@ -105,6 +105,10 @@ func (m *Member) Clone() *Member {
|
|||
return mm
|
||||
}
|
||||
|
||||
func (m *Member) IsStarted() bool {
|
||||
return len(m.Name) != 0
|
||||
}
|
||||
|
||||
func memberStoreKey(id types.ID) string {
|
||||
return path.Join(storeMembersPrefix, id.String())
|
||||
}
|
||||
|
|
|
@ -603,6 +603,12 @@ func (s *EtcdServer) LeaderStats() []byte {
|
|||
func (s *EtcdServer) StoreStats() []byte { return s.store.JsonStats() }
|
||||
|
||||
func (s *EtcdServer) AddMember(ctx context.Context, memb Member) error {
|
||||
if s.cfg.StrictReconfigCheck && !s.cluster.isReadyToAddNewMember() {
|
||||
// If s.cfg.StrictReconfigCheck is false, it means the option -strict-reconfig-check isn't passed to etcd.
|
||||
// In such a case adding a new member is allowed unconditionally
|
||||
return ErrNotEnoughStartedMembers
|
||||
}
|
||||
|
||||
// TODO: move Member to protobuf type
|
||||
b, err := json.Marshal(memb)
|
||||
if err != nil {
|
||||
|
|
|
@ -866,6 +866,7 @@ func TestAddMember(t *testing.T) {
|
|||
store: st,
|
||||
cluster: cl,
|
||||
reqIDGen: idutil.NewGenerator(0, time.Time{}),
|
||||
cfg: &ServerConfig{},
|
||||
}
|
||||
s.start()
|
||||
m := Member{ID: 1234, RaftAttributes: RaftAttributes{PeerURLs: []string{"foo"}}}
|
||||
|
|
Loading…
Reference in New Issue