batcheval: generalize checkNotLearnerReplica

tbg · tbg · commit 31fbc37b1556 · 2019-08-23T11:53:41.000+02:00
This now errors out whenever the replica is not a voter, which is more
robust as new replica types are introduced (which generally should not
automatically become eligible to receive leases).

Release note: None
diff --git a/pkg/storage/batcheval/cmd_lease.go b/pkg/storage/batcheval/cmd_lease.go
@@ -42,12 +42,12 @@ func newFailedLeaseTrigger(isTransfer bool) result.Result {
 	return trigger
 }
 
-func checkNotLearnerReplica(rec EvalContext) error {
+func checkCanReceiveLease(rec EvalContext) error {
 	repDesc, ok := rec.Desc().GetReplicaDescriptor(rec.StoreID())
 	if !ok {
 		return errors.AssertionFailedf(
 			`could not find replica for store %s in %s`, rec.StoreID(), rec.Desc())
-	} else if t := repDesc.GetType(); t == roachpb.ReplicaType_LEARNER {
+	} else if t := repDesc.GetType(); t != roachpb.ReplicaType_VOTER {
 		return errors.Errorf(`cannot transfer lease to replica of type %s`, t)
 	}
 	return nil
diff --git a/pkg/storage/batcheval/cmd_lease_request.go b/pkg/storage/batcheval/cmd_lease_request.go
@@ -47,7 +47,7 @@ func RequestLease(
 	//
 	// If this check is removed at some point, the filtering of learners on the
 	// sending side would have to be removed as well.
-	if err := checkNotLearnerReplica(cArgs.EvalCtx); err != nil {
+	if err := checkCanReceiveLease(cArgs.EvalCtx); err != nil {
 		return newFailedLeaseTrigger(false /* isTransfer */), err
 	}
 
diff --git a/pkg/storage/batcheval/cmd_lease_transfer.go b/pkg/storage/batcheval/cmd_lease_transfer.go
@@ -46,7 +46,7 @@ func TransferLease(
 	//
 	// If this check is removed at some point, the filtering of learners on the
 	// sending side would have to be removed as well.
-	if err := checkNotLearnerReplica(cArgs.EvalCtx); err != nil {
+	if err := checkCanReceiveLease(cArgs.EvalCtx); err != nil {
 		return newFailedLeaseTrigger(true /* isTransfer */), err
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -42,12 +42,12 @@ func newFailedLeaseTrigger(isTransfer bool) result.Result {`
`42`	`42`	`return trigger`
`43`	`43`	`}`
`44`	`44`
`45`		`-func checkNotLearnerReplica(rec EvalContext) error {`
	`45`	`+func checkCanReceiveLease(rec EvalContext) error {`
`46`	`46`	`repDesc, ok := rec.Desc().GetReplicaDescriptor(rec.StoreID())`
`47`	`47`	`if !ok {`
`48`	`48`	`return errors.AssertionFailedf(`
`49`	`49`	`could not find replica for store %s in %s`, rec.StoreID(), rec.Desc())
`50`		`- } else if t := repDesc.GetType(); t == roachpb.ReplicaType_LEARNER {`
	`50`	`+ } else if t := repDesc.GetType(); t != roachpb.ReplicaType_VOTER {`
`51`	`51`	return errors.Errorf(`cannot transfer lease to replica of type %s`, t)
`52`	`52`	`}`
`53`	`53`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ func RequestLease(`
`47`	`47`	`//`
`48`	`48`	`// If this check is removed at some point, the filtering of learners on the`
`49`	`49`	`// sending side would have to be removed as well.`
`50`		`- if err := checkNotLearnerReplica(cArgs.EvalCtx); err != nil {`
	`50`	`+ if err := checkCanReceiveLease(cArgs.EvalCtx); err != nil {`
`51`	`51`	`return newFailedLeaseTrigger(false /* isTransfer */), err`
`52`	`52`	`}`
`53`	`53`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ func TransferLease(`
`46`	`46`	`//`
`47`	`47`	`// If this check is removed at some point, the filtering of learners on the`
`48`	`48`	`// sending side would have to be removed as well.`
`49`		`- if err := checkNotLearnerReplica(cArgs.EvalCtx); err != nil {`
	`49`	`+ if err := checkCanReceiveLease(cArgs.EvalCtx); err != nil {`
`50`	`50`	`return newFailedLeaseTrigger(true /* isTransfer */), err`
`51`	`51`	`}`
`52`	`52`