Skip to content

Commit

Permalink
Merge #47413 #65094
Browse files Browse the repository at this point in the history 
47413: kvserver/reports: handle joint-quorums r=andreimatei a=andreimatei

See individual commits.

65094: bazel: stage all go source files in a temp dir named after the package r=rail,knz a=rickystewart

This works around an issue where Go source file names as stored in the
`cockroach` binary were truncated (e.g., with the leading
`github.com/cockroachdb/cockroach` prefix, or even the entire package
name prefix, removed). This breaks unit tests (#61913) and some other
internal stuff.

We solve this by staging all Go source files during the build in a
temporary directory named after the package. This incurs an additional
I/O cost, but for now while our codebase isn't able to deal with the
differing file names, we can deal with it.

Fixes #64379
See also #64383

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
@andreimatei @rickystewart
3 people committed May 13, 2021
3 parents 5c0b2ec + 39ff800 + 1b78c30 commit a40c777
Show file tree
Hide file tree
Showing 14 changed files with 586 additions and 217 deletions.
2 changes: 1 addition & 1 deletion WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
# Load go bazel tools. This gives us access to the go bazel SDK/toolchains.
git_repository(
name = "io_bazel_rules_go",
commit = "2fe8a6256c818840cc9a10cf3f366d8410437245",
commit = "a94a425d26532613e71f9c7432c19c36302e1f1d",
remote = "https://github.com/cockroachdb/rules_go",
)

Expand Down
13 changes: 9 additions & 4 deletions pkg/kv/kvserver/replica_metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,17 @@ func calcRangeCounter(
// We also compute an estimated per-range count of under-replicated and
// unavailable ranges for each range based on the liveness table.
if rangeCounter {
unavailable = !desc.Replicas().CanMakeProgress(func(rDesc roachpb.ReplicaDescriptor) bool {
return livenessMap[rDesc.NodeID].IsLive
})
neededVoters := GetNeededVoters(numVoters, clusterNodes)
liveVoters := calcLiveVoterReplicas(desc, livenessMap)
neededNonVoters := GetNeededNonVoters(int(numVoters), int(numReplicas-numVoters), clusterNodes)
status := desc.Replicas().ReplicationStatus(func(rDesc roachpb.ReplicaDescriptor) bool {
return livenessMap[rDesc.NodeID].IsLive
},
// neededVoters - we don't care about the under/over-replication
// determinations from the report because it's too magic. We'll do our own
// determination below.
0)
unavailable = !status.Available
liveVoters := calcLiveVoterReplicas(desc, livenessMap)
liveNonVoters := calcLiveNonVoterReplicas(desc, livenessMap)
if neededVoters > liveVoters || neededNonVoters > liveNonVoters {
underreplicated = true
Expand Down
183 changes: 135 additions & 48 deletions pkg/kv/kvserver/reports/constraint_stats_report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
"bytes"
"context"
"fmt"
"regexp"
"sort"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -72,17 +74,17 @@ func TestConformanceReport(t *testing.T) {
},
},
splits: []split{
{key: "/Table/t1", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/1", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/2", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/3", stores: []int{1, 2, 3}},
{key: "/Table/t2", stores: []int{1, 2, 3}},
{key: "/Table/t2/pk", stores: []int{1, 2, 3}},
{key: "/Table/t1", stores: "1 2 3"},
{key: "/Table/t1/pk", stores: "1 2 3"},
{key: "/Table/t1/pk/1", stores: "1 2 3"},
{key: "/Table/t1/pk/2", stores: "1 2 3"},
{key: "/Table/t1/pk/3", stores: "1 2 3"},
{key: "/Table/t2", stores: "1 2 3"},
{key: "/Table/t2/pk", stores: "1 2 3"},
{
// This range is not covered by the db1's zone config and so it won't
// be counted.
key: "/Table/sentinel", stores: []int{1, 2, 3},
key: "/Table/sentinel", stores: "1 2 3",
},
},
nodes: []node{
Expand Down Expand Up @@ -150,20 +152,20 @@ func TestConformanceReport(t *testing.T) {
},
},
splits: []split{
{key: "/Table/t1", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/1", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/2", stores: []int{1, 2, 3}},
{key: "/Table/t1/pk/3", stores: []int{1, 2, 3}},
{key: "/Table/t2", stores: []int{1, 2, 3}},
{key: "/Table/t2/pk", stores: []int{1, 2, 3}},
{key: "/Table/sentinel", stores: []int{1, 2, 3}},
{key: "/Table/t3", stores: []int{1, 2, 3}},
{key: "/Table/t3/pk/100", stores: []int{1, 2, 3}},
{key: "/Table/t3/pk/101", stores: []int{1, 2, 3}},
{key: "/Table/t3/pk/199", stores: []int{1, 2, 3}},
{key: "/Table/t3/pk/200", stores: []int{1, 2, 3}},
{key: "/Table/t4", stores: []int{1, 2, 3}},
{key: "/Table/t1", stores: "1 2 3"},
{key: "/Table/t1/pk", stores: "1 2 3"},
{key: "/Table/t1/pk/1", stores: "1 2 3"},
{key: "/Table/t1/pk/2", stores: "1 2 3"},
{key: "/Table/t1/pk/3", stores: "1 2 3"},
{key: "/Table/t2", stores: "1 2 3"},
{key: "/Table/t2/pk", stores: "1 2 3"},
{key: "/Table/sentinel", stores: "1 2 3"},
{key: "/Table/t3", stores: "1 2 3"},
{key: "/Table/t3/pk/100", stores: "1 2 3"},
{key: "/Table/t3/pk/101", stores: "1 2 3"},
{key: "/Table/t3/pk/199", stores: "1 2 3"},
{key: "/Table/t3/pk/200", stores: "1 2 3"},
{key: "/Table/t4", stores: "1 2 3"},
},
// None of the stores have any attributes.
nodes: []node{
Expand Down Expand Up @@ -235,7 +237,7 @@ func TestConformanceReport(t *testing.T) {
},
},
splits: []split{
{key: "/Table/t1", stores: []int{1, 2}},
{key: "/Table/t1", stores: "1 2"},
},
nodes: []node{
{id: 1, locality: "region=us,dc=dc1", stores: []store{{id: 1}}},
Expand Down Expand Up @@ -469,8 +471,13 @@ func (c constraintEntry) toReportEntry(
}

type split struct {
key string
stores []int
key string
// stores lists the stores that are gonna have replicas for a particular range. It's a space-separated
// list of store ids, with an optional replica type. If the type of replica is missing, it will
// be a voter-full.
// The format is a space-separated list of <storeID><?replica type>. For example:
// "1 2 3 4v 5i 6o 7d 8l" (1,2,3,4 are voter-full, then incoming, outgoing, demoting, learner).
stores string
}

type store struct {
Expand Down Expand Up @@ -738,6 +745,12 @@ type compiledTestCase struct {
objectToZone map[string]ZoneKey
// The inverse of objectToZone.
zoneToObject map[ZoneKey]string

// nodeLocalities maps from each node to its locality.
nodeLocalities map[roachpb.NodeID]roachpb.Locality
// allLocalities is the list of all localities in the cluster, at all
// granularities.
allLocalities map[roachpb.NodeID]map[string]roachpb.Locality
}

// compileTestCase takes the input schema and turns it into a compiledTestCase.
Expand Down Expand Up @@ -795,24 +808,43 @@ func compileTestCase(tc baseReportTestCase) (compiledTestCase, error) {
}
}

keyScanner := keysutils.MakePrettyScannerForNamedTables(tableToID, idxToID)
ranges, err := processSplits(keyScanner, tc.splits)
if err != nil {
return compiledTestCase{}, err
}
var allStores []roachpb.StoreDescriptor
for i, n := range tc.nodes {
// Ensure nodes are not defined more than once.
for j := 0; j < i; j++ {
if tc.nodes[j].id == n.id {
return compiledTestCase{}, errors.Errorf("duplicate node definition: %d", n.id)
}
}

var storeDescs []roachpb.StoreDescriptor
for _, n := range tc.nodes {
_ /* nodeDesc */, sds, err := n.toDescriptors()
if err != nil {
return compiledTestCase{}, err
}
storeDescs = append(storeDescs, sds...)
allStores = append(allStores, sds...)
}

keyScanner := keysutils.MakePrettyScannerForNamedTables(tableToID, idxToID)
ranges, err := processSplits(keyScanner, tc.splits, allStores)
if err != nil {
return compiledTestCase{}, err
}

nodeLocalities := make(map[roachpb.NodeID]roachpb.Locality, len(allStores))
for _, storeDesc := range allStores {
nodeDesc := storeDesc.Node
// Note: We might overwrite the node's localities here. We assume that all
// the stores for a node have the same node descriptor.
nodeLocalities[nodeDesc.NodeID] = nodeDesc.Locality
}
allLocalities := expandLocalities(nodeLocalities)
storeResolver := func(r *roachpb.RangeDescriptor) []roachpb.StoreDescriptor {
stores := make([]roachpb.StoreDescriptor, len(r.Replicas().VoterDescriptors()))
for i, rep := range r.Replicas().VoterDescriptors() {
for _, desc := range storeDescs {
replicas := r.Replicas().FilterToDescriptors(func(_ roachpb.ReplicaDescriptor) bool {
return true
})
stores := make([]roachpb.StoreDescriptor, len(replicas))
for i, rep := range replicas {
for _, desc := range allStores {
if rep.StoreID == desc.StoreID {
stores[i] = desc
break
Expand All @@ -836,12 +868,14 @@ func compileTestCase(tc baseReportTestCase) (compiledTestCase, error) {
objectToZone[v] = k
}
return compiledTestCase{
iter: testRangeIter{ranges: ranges},
cfg: cfg,
resolver: storeResolver,
checker: nodeChecker,
zoneToObject: zoneToObject,
objectToZone: objectToZone,
iter: testRangeIter{ranges: ranges},
cfg: cfg,
resolver: storeResolver,
checker: nodeChecker,
zoneToObject: zoneToObject,
objectToZone: objectToZone,
nodeLocalities: nodeLocalities,
allLocalities: allLocalities,
}, nil
}

Expand Down Expand Up @@ -873,8 +907,18 @@ func generateTableZone(t table, tableDesc descpb.TableDescriptor) (*zonepb.ZoneC
}

func processSplits(
keyScanner keysutil.PrettyScanner, splits []split,
keyScanner keysutil.PrettyScanner, splits []split, stores []roachpb.StoreDescriptor,
) ([]roachpb.RangeDescriptor, error) {

findStore := func(storeID int) (roachpb.StoreDescriptor, error) {
for _, s := range stores {
if s.StoreID == roachpb.StoreID(storeID) {
return s, nil
}
}
return roachpb.StoreDescriptor{}, errors.Errorf("store not found: %d", storeID)
}

ranges := make([]roachpb.RangeDescriptor, len(splits))
var lastKey roachpb.Key
for i, split := range splits {
Expand All @@ -900,13 +944,56 @@ func processSplits(
}

rd := roachpb.RangeDescriptor{
RangeID: roachpb.RangeID(i + 1), // IDs start at 1
StartKey: keys.MustAddr(startKey),
EndKey: keys.MustAddr(endKey),
RangeID: roachpb.RangeID(i + 1), // IDs start at 1
StartKey: keys.MustAddr(startKey),
EndKey: keys.MustAddr(endKey),
NextReplicaID: roachpb.ReplicaID(2), // IDs start at 1, so Next starts at 2.
}
for _, storeID := range split.stores {
rd.AddReplica(roachpb.NodeID(storeID), roachpb.StoreID(storeID), roachpb.VOTER_FULL)

reps := strings.Split(split.stores, " ")
for _, rep := range reps {
if rep == "" {
continue
}
// The format is a numeric storeID, optionally followed by a single character
// indicating the replica type.
re := regexp.MustCompile("([0-9]+)(.?)")
spec := re.FindStringSubmatch(rep)
if len(spec) != 3 {
return nil, errors.Errorf("bad replica spec: %s", rep)
}
storeID, err := strconv.Atoi(spec[1])
if err != nil {
return nil, err
}
storeDesc, err := findStore(storeID)
if err != nil {
return nil, err
}
// Figure out the type of the replica. It's VOTER_FULL by default, unless
// another option was specified.
typ := roachpb.VOTER_FULL
if spec[2] != "" {
replicaType := spec[2]
switch replicaType {
case "v":
typ = roachpb.VOTER_FULL
case "i":
typ = roachpb.VOTER_INCOMING
case "o":
typ = roachpb.VOTER_OUTGOING
case "d":
typ = roachpb.VOTER_DEMOTING_NON_VOTER
case "l":
typ = roachpb.LEARNER
default:
return nil, errors.Errorf("bad replica type: %s", replicaType)
}
}

rd.AddReplica(storeDesc.Node.NodeID, storeDesc.StoreID, typ)
}

ranges[i] = rd
}
return ranges, nil
Expand Down
Loading

0 comments on commit a40c777

Please sign in to comment.