Merge pull request #14272 from serathius/memberID

memberID equals zero in corruption alarm

CHANGELOG/CHANGELOG-3.5.md

@@ -16,6 +16,7 @@ Previous change logs can be found at [CHANGELOG-3.4](https://github.com/etcd-io/
 - Fix [lessor may continue to schedule checkpoint after stepping down leader role](https://github.com/etcd-io/etcd/pull/14087).
 - Fix [Restrict the max size of each WAL entry to the remaining size of the WAL file](https://github.com/etcd-io/etcd/pull/14127).
 - Fix [Protect rangePermCache with a RW lock correctly](https://github.com/etcd-io/etcd/pull/14227)
+- Fix [memberID equals zero in corruption alarm](https://github.com/etcd-io/etcd/pull/14272)
 - 
 ### Other
 - [Bump golang.org/x/crypto to latest version](https://github.com/etcd-io/etcd/pull/13996) to address [CVE-2022-27191](https://github.com/advisories/GHSA-8c26-wmh5-6g9v).

CHANGELOG/CHANGELOG-3.6.md

@@ -73,6 +73,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0).
 - Fix [etcd panic on startup (auth enabled)](https://github.com/etcd-io/etcd/pull/13942)
 - Fix [Defrag unsets backend options](https://github.com/etcd-io/etcd/pull/13679).
 - Fix [Restrict the max size of each WAL entry to the remaining size of the WAL file](https://github.com/etcd-io/etcd/pull/14122).
+- Fix [memberID equals zero in corruption alarm](https://github.com/etcd-io/etcd/pull/14272)
 
 ### tools/benchmark
 

server/etcdserver/corrupt.go

@@ -55,7 +55,7 @@ type Hasher interface {
 	MemberId() types.ID
 	PeerHashByRev(int64) []*peerHashKVResp
 	LinearizableReadNotify(context.Context) error
-	TriggerCorruptAlarm(uint64)
+	TriggerCorruptAlarm(types.ID)
 }
 
 func newCorruptionChecker(lg *zap.Logger, s *EtcdServer, storage mvcc.HashStorage) *corruptionChecker {
@@ -78,7 +78,7 @@ func (h hasherAdapter) PeerHashByRev(rev int64) []*peerHashKVResp {
 	return h.EtcdServer.getPeerHashKVs(rev)
 }
 
-func (h hasherAdapter) TriggerCorruptAlarm(memberID uint64) {
+func (h hasherAdapter) TriggerCorruptAlarm(memberID types.ID) {
 	h.EtcdServer.triggerCorruptAlarm(memberID)
 }
 
@@ -184,7 +184,7 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 	}
 
 	alarmed := false
-	mismatch := func(id uint64) {
+	mismatch := func(id types.ID) {
 		if alarmed {
 			return
 		}
@@ -202,7 +202,7 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 			zap.Int64("compact-revision-2", h2.CompactRevision),
 			zap.Uint32("hash-2", h2.Hash),
 		)
-		mismatch(uint64(cm.hasher.MemberId()))
+		mismatch(cm.hasher.MemberId())
 	}
 
 	checkedCount := 0
@@ -211,7 +211,6 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 			continue
 		}
 		checkedCount++
-		id := p.resp.Header.MemberId
 
 		// leader expects follower's latest revision less than or equal to leader's
 		if p.resp.Header.Revision > rev2 {
@@ -219,9 +218,9 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 				"revision from follower must be less than or equal to leader's",
 				zap.Int64("leader-revision", rev2),
 				zap.Int64("follower-revision", p.resp.Header.Revision),
-				zap.String("follower-peer-id", types.ID(id).String()),
+				zap.String("follower-peer-id", p.id.String()),
 			)
-			mismatch(id)
+			mismatch(p.id)
 		}
 
 		// leader expects follower's latest compact revision less than or equal to leader's
@@ -230,9 +229,9 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 				"compact revision from follower must be less than or equal to leader's",
 				zap.Int64("leader-compact-revision", h2.CompactRevision),
 				zap.Int64("follower-compact-revision", p.resp.CompactRevision),
-				zap.String("follower-peer-id", types.ID(id).String()),
+				zap.String("follower-peer-id", p.id.String()),
 			)
-			mismatch(id)
+			mismatch(p.id)
 		}
 
 		// follower's compact revision is leader's old one, then hashes must match
@@ -243,9 +242,9 @@ func (cm *corruptionChecker) PeriodicCheck() error {
 				zap.Uint32("leader-hash", h.Hash),
 				zap.Int64("follower-compact-revision", p.resp.CompactRevision),
 				zap.Uint32("follower-hash", p.resp.Hash),
-				zap.String("follower-peer-id", types.ID(id).String()),
+				zap.String("follower-peer-id", p.id.String()),
 			)
-			mismatch(id)
+			mismatch(p.id)
 		}
 	}
 	cm.lg.Info("finished peer corruption check", zap.Int("number-of-peers-checked", checkedCount))
@@ -272,7 +271,7 @@ func (cm *corruptionChecker) CompactHashCheck() {
 
 			// follower's compact revision is leader's old one, then hashes must match
 			if p.resp.Hash != hash.Hash {
-				cm.hasher.TriggerCorruptAlarm(uint64(p.id))
+				cm.hasher.TriggerCorruptAlarm(p.id)
 				cm.lg.Error("failed compaction hash check",
 					zap.Int64("revision", hash.Revision),
 					zap.Int64("leader-compact-revision", hash.CompactRevision),
@@ -330,9 +329,9 @@ func (cm *corruptionChecker) uncheckedRevisions() []mvcc.KeyValueHash {
 	return hashes
 }
 
-func (s *EtcdServer) triggerCorruptAlarm(id uint64) {
+func (s *EtcdServer) triggerCorruptAlarm(id types.ID) {
 	a := &pb.AlarmRequest{
-		MemberID: id,
+		MemberID: uint64(id),
 		Action:   pb.AlarmRequest_ACTIVATE,
 		Alarm:    pb.AlarmType_CORRUPT,
 	}

server/etcdserver/corrupt_test.go

@@ -161,7 +161,7 @@ func TestPeriodicCheck(t *testing.T) {
 		{
 			name: "Peer with newer revision",
 			hasher: fakeHasher{
-				peerHashes: []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1, MemberId: 42}}}},
+				peerHashes: []*peerHashKVResp{{peerInfo: peerInfo{id: 42}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1}}}},
 			},
 			expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(42)"},
 			expectCorrupt: true,
@@ -169,7 +169,7 @@ func TestPeriodicCheck(t *testing.T) {
 		{
 			name: "Peer with newer compact revision",
 			hasher: fakeHasher{
-				peerHashes: []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 88}, CompactRevision: 2}}},
+				peerHashes: []*peerHashKVResp{{peerInfo: peerInfo{id: 88}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}}},
 			},
 			expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(88)"},
 			expectCorrupt: true,
@@ -186,7 +186,7 @@ func TestPeriodicCheck(t *testing.T) {
 			name: "Peer with different hash and same compact revision as first local",
 			hasher: fakeHasher{
 				hashByRevResponses: []hashByRev{{hash: mvcc.KeyValueHash{Hash: 1, CompactRevision: 1}, revision: 1}, {hash: mvcc.KeyValueHash{Hash: 2, CompactRevision: 2}, revision: 2}},
-				peerHashes:         []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1, MemberId: 666}, CompactRevision: 1, Hash: 2}}},
+				peerHashes:         []*peerHashKVResp{{peerInfo: peerInfo{id: 666}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1}, CompactRevision: 1, Hash: 2}}},
 			},
 			expectActions: []string{"HashByRev(0)", "PeerHashByRev(1)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(666)"},
 			expectCorrupt: true,
@@ -195,8 +195,8 @@ func TestPeriodicCheck(t *testing.T) {
 			name: "Multiple corrupted peers trigger one alarm",
 			hasher: fakeHasher{
 				peerHashes: []*peerHashKVResp{
-					{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 88}, CompactRevision: 2}},
+					{peerInfo: peerInfo{id: 88}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}},
-					{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 89}, CompactRevision: 2}},
+					{peerInfo: peerInfo{id: 89}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}},
 				},
 			},
 			expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(88)"},
@@ -371,7 +371,7 @@ func (f *fakeHasher) LinearizableReadNotify(ctx context.Context) error {
 	return f.linearizableReadNotify
 }
 
-func (f *fakeHasher) TriggerCorruptAlarm(memberId uint64) {
+func (f *fakeHasher) TriggerCorruptAlarm(memberId types.ID) {
 	f.actions = append(f.actions, fmt.Sprintf("TriggerCorruptAlarm(%d)", memberId))
 	f.alarmTriggered = true
 }

tests/e2e/corrupt_test.go

@@ -121,6 +121,15 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) {
 		assert.NoError(t, err, "error on put")
 	}
 
+	members, err := cc.MemberList()
+	assert.NoError(t, err, "error on member list")
+	var memberID uint64
+	for _, m := range members.Members {
+		if m.Name == epc.Procs[0].Config().Name {
+			memberID = m.ID
+		}
+	}
+	assert.NotZero(t, memberID, "member not found")
 	epc.Procs[0].Stop()
 	err = testutil.CorruptBBolt(datadir.ToBackendFileName(epc.Procs[0].Config().DataDirPath))
 	assert.NoError(t, err)
@@ -130,8 +139,7 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) {
 	time.Sleep(checkTime * 11 / 10)
 	alarmResponse, err := cc.AlarmList()
 	assert.NoError(t, err, "error on alarm list")
-	// TODO: Investigate why MemberID is 0?
+	assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: memberID}}, alarmResponse.Alarms)
-	assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: 0}}, alarmResponse.Alarms)
 }
 
 func TestCompactHashCheckDetectCorruption(t *testing.T) {

tests/integration/corrupt_test.go

@@ -97,8 +97,7 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) {
 	time.Sleep(50 * time.Millisecond)
 	alarmResponse, err := cc.AlarmList(ctx)
 	assert.NoError(t, err, "error on alarm list")
-	// TODO: Investigate why MemberID is 0?
+	assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: uint64(clus.Members[0].ID())}}, alarmResponse.Alarms)
-	assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: 0}}, alarmResponse.Alarms)
 }
 
 func TestCompactHashCheck(t *testing.T) {