Skip to content

Commit

Permalink
liveness: move IsLiveMap type
Browse files Browse the repository at this point in the history
We'll depend on just this directly in a future commit, without wanting to
depend on the much larger liveness package. Instead of introducing a
standalone package for it, we'll just re-use the thin livenesspb package
instead.

Release note: None
  • Loading branch information
irfansharif committed Nov 24, 2022
1 parent 14f61c5 commit 6ed9148
Show file tree
Hide file tree
Showing 15 changed files with 97 additions and 97 deletions.
16 changes: 3 additions & 13 deletions pkg/kv/kvserver/liveness/liveness.go
Original file line number Diff line number Diff line change
Expand Up @@ -1025,21 +1025,11 @@ func (nl *NodeLiveness) SelfEx() (_ Record, ok bool) {
return nl.getLivenessLocked(nl.gossip.NodeID.Get())
}

// IsLiveMapEntry encapsulates data about current liveness for a
// node.
type IsLiveMapEntry struct {
livenesspb.Liveness
IsLive bool
}

// IsLiveMap is a type alias for a map from NodeID to IsLiveMapEntry.
type IsLiveMap map[roachpb.NodeID]IsLiveMapEntry

// GetIsLiveMap returns a map of nodeID to boolean liveness status of
// each node. This excludes nodes that were removed completely (dead +
// decommissioning).
func (nl *NodeLiveness) GetIsLiveMap() IsLiveMap {
lMap := IsLiveMap{}
func (nl *NodeLiveness) GetIsLiveMap() livenesspb.IsLiveMap {
lMap := livenesspb.IsLiveMap{}
nl.mu.RLock()
defer nl.mu.RUnlock()
now := nl.clock.Now().GoTime()
Expand All @@ -1049,7 +1039,7 @@ func (nl *NodeLiveness) GetIsLiveMap() IsLiveMap {
// This is a node that was completely removed. Skip over it.
continue
}
lMap[nID] = IsLiveMapEntry{
lMap[nID] = livenesspb.IsLiveMapEntry{
Liveness: l.Liveness,
IsLive: isLive,
}
Expand Down
1 change: 1 addition & 0 deletions pkg/kv/kvserver/liveness/livenesspb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb",
visibility = ["//visibility:public"],
deps = [
"//pkg/roachpb",
"//pkg/util/timeutil",
"@org_golang_google_grpc//codes",
"@org_golang_google_grpc//status",
Expand Down
11 changes: 11 additions & 0 deletions pkg/kv/kvserver/liveness/livenesspb/liveness.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -132,3 +133,13 @@ func ValidateTransition(old, new Liveness) error {

return nil
}

// IsLiveMapEntry encapsulates data about current liveness for a
// node.
type IsLiveMapEntry struct {
Liveness
IsLive bool
}

// IsLiveMap is a type alias for a map from NodeID to IsLiveMapEntry.
type IsLiveMap map[roachpb.NodeID]IsLiveMapEntry
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/node_liveness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ func TestNodeLivenessGetIsLiveMap(t *testing.T) {
l1, _ := nl.GetLiveness(1)
l2, _ := nl.GetLiveness(2)
l3, _ := nl.GetLiveness(3)
expectedLMap := liveness.IsLiveMap{
expectedLMap := livenesspb.IsLiveMap{
1: {Liveness: l1.Liveness, IsLive: true},
2: {Liveness: l2.Liveness, IsLive: true},
3: {Liveness: l3.Liveness, IsLive: true},
Expand Down Expand Up @@ -705,7 +705,7 @@ func TestNodeLivenessGetIsLiveMap(t *testing.T) {
l1, _ = nl.GetLiveness(1)
l2, _ = nl.GetLiveness(2)
l3, _ = nl.GetLiveness(3)
expectedLMap = liveness.IsLiveMap{
expectedLMap = livenesspb.IsLiveMap{
1: {Liveness: l1.Liveness, IsLive: true},
2: {Liveness: l2.Liveness, IsLive: false},
3: {Liveness: l3.Liveness, IsLive: false},
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/replica_circuit_breaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings"
Expand Down Expand Up @@ -239,7 +239,7 @@ func replicaUnavailableError(
err error,
desc *roachpb.RangeDescriptor,
replDesc roachpb.ReplicaDescriptor,
lm liveness.IsLiveMap,
lm livenesspb.IsLiveMap,
rs *raft.Status,
closedTS hlc.Timestamp,
) error {
Expand Down Expand Up @@ -280,7 +280,7 @@ func (r *Replica) replicaUnavailableError(err error) error {
desc := r.Desc()
replDesc, _ := desc.GetReplicaDescriptor(r.store.StoreID())

isLiveMap, _ := r.store.livenessMap.Load().(liveness.IsLiveMap)
isLiveMap, _ := r.store.livenessMap.Load().(livenesspb.IsLiveMap)
ct := r.GetCurrentClosedTimestamp(context.Background())
return replicaUnavailableError(err, desc, replDesc, isLiveMap, r.RaftStatus(), ct)
}
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/replica_circuit_breaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"testing"
"time"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
Expand All @@ -36,8 +36,8 @@ func TestReplicaUnavailableError(t *testing.T) {
repls.AddReplica(roachpb.ReplicaDescriptor{NodeID: 1, StoreID: 10, ReplicaID: 100})
repls.AddReplica(roachpb.ReplicaDescriptor{NodeID: 2, StoreID: 20, ReplicaID: 200})
desc := roachpb.NewRangeDescriptor(10, roachpb.RKey("a"), roachpb.RKey("z"), repls)
lm := liveness.IsLiveMap{
1: liveness.IsLiveMapEntry{IsLive: true},
lm := livenesspb.IsLiveMap{
1: livenesspb.IsLiveMapEntry{IsLive: true},
}
ts, err := time.Parse("2006-01-02 15:04:05", "2006-01-02 15:04:05")
require.NoError(t, err)
Expand Down
16 changes: 8 additions & 8 deletions pkg/kv/kvserver/replica_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/allocatorimpl"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"go.etcd.io/etcd/raft/v3"
Expand Down Expand Up @@ -60,7 +60,7 @@ type ReplicaMetrics struct {

// Metrics returns the current metrics for the replica.
func (r *Replica) Metrics(
ctx context.Context, now hlc.ClockTimestamp, livenessMap liveness.IsLiveMap, clusterNodes int,
ctx context.Context, now hlc.ClockTimestamp, livenessMap livenesspb.IsLiveMap, clusterNodes int,
) ReplicaMetrics {
r.store.unquiescedReplicas.Lock()
_, ticking := r.store.unquiescedReplicas.m[r.RangeID]
Expand Down Expand Up @@ -107,7 +107,7 @@ func (r *Replica) Metrics(
type calcReplicaMetricsInput struct {
raftCfg *base.RaftConfig
conf roachpb.SpanConfig
livenessMap liveness.IsLiveMap
livenessMap livenesspb.IsLiveMap
clusterNodes int
desc *roachpb.RangeDescriptor
raftStatus *raft.Status
Expand Down Expand Up @@ -194,7 +194,7 @@ func calcRangeCounter(
storeID roachpb.StoreID,
desc *roachpb.RangeDescriptor,
leaseStatus kvserverpb.LeaseStatus,
livenessMap liveness.IsLiveMap,
livenessMap livenesspb.IsLiveMap,
numVoters, numReplicas int32,
clusterNodes int,
) (rangeCounter, unavailable, underreplicated, overreplicated bool) {
Expand Down Expand Up @@ -240,17 +240,17 @@ func calcRangeCounter(
// replica is determined by checking its node in the provided liveness map. This
// method is used when indicating under-replication so only voter replicas are
// considered.
func calcLiveVoterReplicas(desc *roachpb.RangeDescriptor, livenessMap liveness.IsLiveMap) int {
func calcLiveVoterReplicas(desc *roachpb.RangeDescriptor, livenessMap livenesspb.IsLiveMap) int {
return calcLiveReplicas(desc.Replicas().VoterDescriptors(), livenessMap)
}

// calcLiveNonVoterReplicas returns a count of the live non-voter replicas; a live
// replica is determined by checking its node in the provided liveness map.
func calcLiveNonVoterReplicas(desc *roachpb.RangeDescriptor, livenessMap liveness.IsLiveMap) int {
func calcLiveNonVoterReplicas(desc *roachpb.RangeDescriptor, livenessMap livenesspb.IsLiveMap) int {
return calcLiveReplicas(desc.Replicas().NonVoterDescriptors(), livenessMap)
}

func calcLiveReplicas(repls []roachpb.ReplicaDescriptor, livenessMap liveness.IsLiveMap) int {
func calcLiveReplicas(repls []roachpb.ReplicaDescriptor, livenessMap livenesspb.IsLiveMap) int {
var live int
for _, rd := range repls {
if livenessMap[rd.NodeID].IsLive {
Expand All @@ -263,7 +263,7 @@ func calcLiveReplicas(repls []roachpb.ReplicaDescriptor, livenessMap liveness.Is
// calcBehindCount returns a total count of log entries that follower replicas
// are behind. This can only be computed on the raft leader.
func calcBehindCount(
raftStatus *raft.Status, desc *roachpb.RangeDescriptor, livenessMap liveness.IsLiveMap,
raftStatus *raft.Status, desc *roachpb.RangeDescriptor, livenessMap livenesspb.IsLiveMap,
) int64 {
var behindCount int64
for _, rd := range desc.Replicas().Descriptors() {
Expand Down
54 changes: 27 additions & 27 deletions pkg/kv/kvserver/replica_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -55,8 +55,8 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {
}))

{
ctr, down, under, over := calcRangeCounter(1100, threeVotersAndSingleNonVoter, leaseStatus, liveness.IsLiveMap{
1000: liveness.IsLiveMapEntry{IsLive: true}, // by NodeID
ctr, down, under, over := calcRangeCounter(1100, threeVotersAndSingleNonVoter, leaseStatus, livenesspb.IsLiveMap{
1000: livenesspb.IsLiveMapEntry{IsLive: true}, // by NodeID
}, 3 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)

require.True(t, ctr)
Expand All @@ -66,8 +66,8 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {
}

{
ctr, down, under, over := calcRangeCounter(1000, threeVotersAndSingleNonVoter, leaseStatus, liveness.IsLiveMap{
1000: liveness.IsLiveMapEntry{IsLive: false},
ctr, down, under, over := calcRangeCounter(1000, threeVotersAndSingleNonVoter, leaseStatus, livenesspb.IsLiveMap{
1000: livenesspb.IsLiveMapEntry{IsLive: false},
}, 3 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)

// Does not confuse a non-live entry for a live one. In other words,
Expand All @@ -79,11 +79,11 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {
}

{
ctr, down, under, over := calcRangeCounter(11, threeVotersAndSingleNonVoter, leaseStatus, liveness.IsLiveMap{
10: liveness.IsLiveMapEntry{IsLive: true},
100: liveness.IsLiveMapEntry{IsLive: true},
1000: liveness.IsLiveMapEntry{IsLive: true},
2000: liveness.IsLiveMapEntry{IsLive: true},
ctr, down, under, over := calcRangeCounter(11, threeVotersAndSingleNonVoter, leaseStatus, livenesspb.IsLiveMap{
10: livenesspb.IsLiveMapEntry{IsLive: true},
100: livenesspb.IsLiveMapEntry{IsLive: true},
1000: livenesspb.IsLiveMapEntry{IsLive: true},
2000: livenesspb.IsLiveMapEntry{IsLive: true},
}, 3 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)

require.True(t, ctr)
Expand All @@ -94,11 +94,11 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {

{
// Single non-voter dead
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, liveness.IsLiveMap{
10: liveness.IsLiveMapEntry{IsLive: true},
100: liveness.IsLiveMapEntry{IsLive: true},
1000: liveness.IsLiveMapEntry{IsLive: false},
2000: liveness.IsLiveMapEntry{IsLive: true},
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, livenesspb.IsLiveMap{
10: livenesspb.IsLiveMapEntry{IsLive: true},
100: livenesspb.IsLiveMapEntry{IsLive: true},
1000: livenesspb.IsLiveMapEntry{IsLive: false},
2000: livenesspb.IsLiveMapEntry{IsLive: true},
}, 1 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)

require.True(t, ctr)
Expand All @@ -109,11 +109,11 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {

{
// All non-voters are dead, but range is not unavailable
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, liveness.IsLiveMap{
10: liveness.IsLiveMapEntry{IsLive: true},
100: liveness.IsLiveMapEntry{IsLive: false},
1000: liveness.IsLiveMapEntry{IsLive: false},
2000: liveness.IsLiveMapEntry{IsLive: false},
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, livenesspb.IsLiveMap{
10: livenesspb.IsLiveMapEntry{IsLive: true},
100: livenesspb.IsLiveMapEntry{IsLive: false},
1000: livenesspb.IsLiveMapEntry{IsLive: false},
2000: livenesspb.IsLiveMapEntry{IsLive: false},
}, 1 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)

require.True(t, ctr)
Expand All @@ -124,11 +124,11 @@ func TestCalcRangeCounterIsLiveMap(t *testing.T) {

{
// More non-voters than needed
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, liveness.IsLiveMap{
10: liveness.IsLiveMapEntry{IsLive: true},
100: liveness.IsLiveMapEntry{IsLive: true},
1000: liveness.IsLiveMapEntry{IsLive: true},
2000: liveness.IsLiveMapEntry{IsLive: true},
ctr, down, under, over := calcRangeCounter(11, oneVoterAndThreeNonVoters, leaseStatus, livenesspb.IsLiveMap{
10: livenesspb.IsLiveMapEntry{IsLive: true},
100: livenesspb.IsLiveMapEntry{IsLive: true},
1000: livenesspb.IsLiveMapEntry{IsLive: true},
2000: livenesspb.IsLiveMapEntry{IsLive: true},
}, 1 /* numVoters */, 3 /* numReplicas */, 4 /* clusterNodes */)

require.True(t, ctr)
Expand Down Expand Up @@ -238,9 +238,9 @@ func TestCalcRangeCounterLeaseHolder(t *testing.T) {

for _, tc := range testcases {
t.Run(tc.desc, func(t *testing.T) {
livenessMap := liveness.IsLiveMap{}
livenessMap := livenesspb.IsLiveMap{}
for _, nodeID := range tc.liveNodes {
livenessMap[nodeID] = liveness.IsLiveMapEntry{IsLive: true}
livenessMap[nodeID] = livenesspb.IsLiveMapEntry{IsLive: true}
}
ctr, _, _, _ := calcRangeCounter(tc.storeID, rangeDesc, tc.leaseStatus, livenessMap,
3 /* numVoters */, 4 /* numReplicas */, 4 /* clusterNodes */)
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/replica_proposal_buf.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/tracker"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftutil"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
Expand Down Expand Up @@ -1239,7 +1239,7 @@ func (rp *replicaProposer) ownsValidLease(ctx context.Context, now hlc.ClockTime

func (rp *replicaProposer) shouldCampaignOnRedirect(raftGroup proposerRaft) bool {
r := (*Replica)(rp)
livenessMap, _ := r.store.livenessMap.Load().(liveness.IsLiveMap)
livenessMap, _ := r.store.livenessMap.Load().(livenesspb.IsLiveMap)
return shouldCampaignOnLeaseRequestRedirect(
raftGroup.BasicStatus(),
livenessMap,
Expand Down
10 changes: 5 additions & 5 deletions pkg/kv/kvserver/replica_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvadmission"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/logstore"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/uncertainty"
Expand Down Expand Up @@ -1093,7 +1093,7 @@ func maybeFatalOnRaftReadyErr(ctx context.Context, err error) (removed bool) {
// tick the Raft group, returning true if the raft group exists and should
// be queued for Ready processing; false otherwise.
func (r *Replica) tick(
ctx context.Context, livenessMap liveness.IsLiveMap, ioThresholdMap *ioThresholdMap,
ctx context.Context, livenessMap livenesspb.IsLiveMap, ioThresholdMap *ioThresholdMap,
) (bool, error) {
r.unreachablesMu.Lock()
remotes := r.unreachablesMu.remotes
Expand Down Expand Up @@ -1804,7 +1804,7 @@ func shouldCampaignOnWake(
leaseStatus kvserverpb.LeaseStatus,
storeID roachpb.StoreID,
raftStatus raft.BasicStatus,
livenessMap liveness.IsLiveMap,
livenessMap livenesspb.IsLiveMap,
desc *roachpb.RangeDescriptor,
requiresExpiringLease bool,
) bool {
Expand Down Expand Up @@ -1863,7 +1863,7 @@ func (r *Replica) maybeCampaignOnWakeLocked(ctx context.Context) {

leaseStatus := r.leaseStatusAtRLocked(ctx, r.store.Clock().NowAsClockTimestamp())
raftStatus := r.mu.internalRaftGroup.BasicStatus()
livenessMap, _ := r.store.livenessMap.Load().(liveness.IsLiveMap)
livenessMap, _ := r.store.livenessMap.Load().(livenesspb.IsLiveMap)
if shouldCampaignOnWake(leaseStatus, r.store.StoreID(), raftStatus, livenessMap, r.descRLocked(), r.requiresExpiringLeaseRLocked()) {
r.campaignLocked(ctx)
}
Expand All @@ -1887,7 +1887,7 @@ func (r *Replica) maybeCampaignOnWakeLocked(ctx context.Context) {
// become leader and can proceed with a future attempt to acquire the lease.
func shouldCampaignOnLeaseRequestRedirect(
raftStatus raft.BasicStatus,
livenessMap liveness.IsLiveMap,
livenessMap livenesspb.IsLiveMap,
desc *roachpb.RangeDescriptor,
requiresExpiringLease bool,
now hlc.Timestamp,
Expand Down
Loading

0 comments on commit 6ed9148

Please sign in to comment.