Skip to content

Commit

Permalink
kvserver: remove all test instances of empty liveness records
Browse files Browse the repository at this point in the history
In a future commit we'll introduce assertions that this is no longer
possible. Let's update our tests to be more representative of inputs
we'd normally expect.

Release note: None
  • Loading branch information
irfansharif authored and tbg committed Sep 30, 2020
1 parent 63d7e41 commit 12aacec
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 7 deletions.
34 changes: 28 additions & 6 deletions pkg/kv/kvserver/node_liveness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
"github.com/gogo/protobuf/proto"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -481,9 +482,10 @@ func TestNodeLivenessRestart(t *testing.T) {
// seeing the liveness record properly gossiped at store startup.
var expKeys []string
for _, g := range mtc.gossips {
key := gossip.MakeNodeLivenessKey(g.NodeID.Get())
nodeID := g.NodeID.Get()
key := gossip.MakeNodeLivenessKey(nodeID)
expKeys = append(expKeys, key)
if err := g.AddInfoProto(key, &kvserverpb.Liveness{}, 0); err != nil {
if err := g.AddInfoProto(key, &kvserverpb.Liveness{NodeID: nodeID}, 0); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -612,7 +614,15 @@ func TestNodeLivenessGetIsLiveMap(t *testing.T) {

// Advance the clock but only heartbeat node 0.
mtc.manualClock.Increment(mtc.nodeLivenesses[0].GetLivenessThreshold().Nanoseconds() + 1)
liveness, _ := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
var liveness kvserver.LivenessRecord
testutils.SucceedsSoon(t, func() error {
livenessRec, err := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
if err != nil {
return err
}
liveness = livenessRec
return nil
})

testutils.SucceedsSoon(t, func() error {
if err := mtc.nodeLivenesses[0].Heartbeat(context.Background(), liveness.Liveness); err != nil {
Expand Down Expand Up @@ -668,7 +678,15 @@ func TestNodeLivenessGetLivenesses(t *testing.T) {

// Advance the clock but only heartbeat node 0.
mtc.manualClock.Increment(mtc.nodeLivenesses[0].GetLivenessThreshold().Nanoseconds() + 1)
liveness, _ := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
var liveness kvserver.LivenessRecord
testutils.SucceedsSoon(t, func() error {
livenessRec, err := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
if err != nil {
return err
}
liveness = livenessRec
return nil
})
if err := mtc.nodeLivenesses[0].Heartbeat(context.Background(), liveness.Liveness); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -791,7 +809,9 @@ func TestNodeLivenessSetDraining(t *testing.T) {
// Verify success on failed update of a liveness record that already has the
// given draining setting.
if err := mtc.nodeLivenesses[drainingNodeIdx].SetDrainingInternal(
ctx, kvserver.LivenessRecord{}, false,
ctx, kvserver.LivenessRecord{Liveness: kvserverpb.Liveness{
NodeID: drainingNodeID,
}}, false,
); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1079,8 +1099,10 @@ func testNodeLivenessSetDecommissioning(t *testing.T, decommissionNodeIdx int) {

// Verify success on failed update of a liveness record that already has the
// given decommissioning setting.
oldLivenessRec, err := callerNodeLiveness.GetLiveness(nodeID)
assert.Nil(t, err)
if _, err := callerNodeLiveness.SetDecommissioningInternal(
ctx, nodeID, kvserver.LivenessRecord{}, kvserverpb.MembershipStatus_ACTIVE,
ctx, nodeID, oldLivenessRec, kvserverpb.MembershipStatus_ACTIVE,
); err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/node_liveness_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func TestShouldReplaceLiveness(t *testing.T) {
}{
{
// Epoch update only.
kvserverpb.Liveness{},
l(1, hlc.Timestamp{}, false, "active"),
l(2, hlc.Timestamp{}, false, "active"),
yes,
},
{
Expand Down

0 comments on commit 12aacec

Please sign in to comment.