Skip to content

Commit

Permalink
storage: disallow nil via types rather than comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@tamird
tamird committed May 9, 2017
1 parent 66f1f96 commit c293630
Show file tree
Hide file tree
Showing 19 changed files with 203 additions and 258 deletions.
11 changes: 5 additions & 6 deletions pkg/roachpb/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,19 @@ type Request interface {
// Implementors return the previous lease at the time the request
// was proposed.
type leaseRequestor interface {
// The returned Lease is never nil; returned by pointer to save a copy.
prevLease() *Lease
prevLease() Lease
}

var _ leaseRequestor = &RequestLeaseRequest{}

func (rlr *RequestLeaseRequest) prevLease() *Lease {
return &rlr.PrevLease
func (rlr *RequestLeaseRequest) prevLease() Lease {
return rlr.PrevLease
}

var _ leaseRequestor = &TransferLeaseRequest{}

func (tlr *TransferLeaseRequest) prevLease() *Lease {
return &tlr.PrevLease
func (tlr *TransferLeaseRequest) prevLease() Lease {
return tlr.PrevLease
}

// Response is an interface for RPC responses.
Expand Down
2 changes: 1 addition & 1 deletion pkg/roachpb/batch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (ba *BatchRequest) IsSinglePushTxnRequest() bool {
// GetPrevLeaseForLeaseRequest returns the previous lease, at the time
// of proposal, for a request lease or transfer lease request. If the
// batch does not contain a single lease request, this method will panic.
func (ba *BatchRequest) GetPrevLeaseForLeaseRequest() *Lease {
func (ba *BatchRequest) GetPrevLeaseForLeaseRequest() Lease {
return ba.Requests[0].GetInner().(leaseRequestor).prevLease()
}

Expand Down
147 changes: 67 additions & 80 deletions pkg/server/debug_range.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,65 +386,54 @@ func (d *debugRangeData) postProcessing() {
Title: raftState,
Value: raftState,
}
if info.State.Lease != nil {
var leaseClass string
if info.State.Lease.Replica.ReplicaID == sourceReplicaID {
leaseClass = debugRangeClassLeaseHolder
} else {
leaseClass = debugRangeClassLeaseFollower
}
d.Results[debugRangeHeaderLeaseHolder][info.SourceStoreID] = &debugRangeOutput{
Class: leaseClass,
Title: info.State.Lease.Replica.ReplicaID.String(),
Value: info.State.Lease.Replica.ReplicaID.String(),
}
var leaseTypeValue string
switch info.State.Lease.Type() {
case roachpb.LeaseNone:
leaseTypeValue = debugRangeValueLeaseNone
case roachpb.LeaseEpoch:
leaseTypeValue = debugRangeValueLeaseEpoch
case roachpb.LeaseExpiration:
leaseTypeValue = debugRangeValueLeaseExpiration
}
d.Results[debugRangeHeaderLeaseType][info.SourceStoreID] = &debugRangeOutput{
Title: leaseTypeValue,
Value: leaseTypeValue,
}
var epoch string
if info.State.Lease.Epoch != nil {
epoch = strconv.FormatInt(*info.State.Lease.Epoch, 10)
} else {
epoch = debugRangeValueEmpty
}
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID] = &debugRangeOutput{
Title: epoch,
Value: epoch,
}
start := convertTimestamp(info.State.Lease.Start)
d.Results[debugRangeHeaderLeaseStart][info.SourceStoreID] = &debugRangeOutput{
Title: fmt.Sprintf("%s\n%s", start, info.State.Lease.Start),
Value: start,
}
var expiration string
if info.State.Lease.Expiration.WallTime == 0 {
expiration = debugRangeValueEmpty
} else {
expiration = convertTimestamp(info.State.Lease.Expiration)
}
d.Results[debugRangeHeaderLeaseExpiration][info.SourceStoreID] = &debugRangeOutput{
Title: fmt.Sprintf("%s\n%s", expiration, info.State.Lease.Expiration),
Value: expiration,
}
var leaseClass string
if info.State.Lease.Replica.ReplicaID == sourceReplicaID {
leaseClass = debugRangeClassLeaseHolder
} else {
leaseClass = debugRangeClassLeaseFollower
}
d.Results[debugRangeHeaderLeaseHolder][info.SourceStoreID] = &debugRangeOutput{
Class: leaseClass,
Title: info.State.Lease.Replica.ReplicaID.String(),
Value: info.State.Lease.Replica.ReplicaID.String(),
}
var leaseTypeValue string
switch info.State.Lease.Type() {
case roachpb.LeaseNone:
leaseTypeValue = debugRangeValueLeaseNone
case roachpb.LeaseEpoch:
leaseTypeValue = debugRangeValueLeaseEpoch
case roachpb.LeaseExpiration:
leaseTypeValue = debugRangeValueLeaseExpiration
}
d.Results[debugRangeHeaderLeaseType][info.SourceStoreID] = &debugRangeOutput{
Title: leaseTypeValue,
Value: leaseTypeValue,
}
var epoch string
if info.State.Lease.Epoch != nil {
epoch = strconv.FormatInt(*info.State.Lease.Epoch, 10)
} else {
d.Results[debugRangeHeaderLeaseHolder][info.SourceStoreID] =
&debugRangeOutput{Value: debugRangeValueEmpty}
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID] =
&debugRangeOutput{Value: debugRangeValueEmpty}
d.Results[debugRangeHeaderLeaseStart][info.SourceStoreID] =
&debugRangeOutput{Value: debugRangeValueEmpty}
d.Results[debugRangeHeaderLeaseExpiration][info.SourceStoreID] =
&debugRangeOutput{Value: debugRangeValueEmpty}
epoch = debugRangeValueEmpty
}
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID] = &debugRangeOutput{
Title: epoch,
Value: epoch,
}
start := convertTimestamp(info.State.Lease.Start)
d.Results[debugRangeHeaderLeaseStart][info.SourceStoreID] = &debugRangeOutput{
Title: fmt.Sprintf("%s\n%s", start, info.State.Lease.Start),
Value: start,
}
var expiration string
if info.State.Lease.Expiration.WallTime == 0 {
expiration = debugRangeValueEmpty
} else {
expiration = convertTimestamp(info.State.Lease.Expiration)
}
d.Results[debugRangeHeaderLeaseExpiration][info.SourceStoreID] = &debugRangeOutput{
Title: fmt.Sprintf("%s\n%s", expiration, info.State.Lease.Expiration),
Value: expiration,
}
d.Results[debugRangeHeaderLeaseAppliedIndex][info.SourceStoreID] = &debugRangeOutput{
Title: strconv.FormatUint(info.State.LeaseAppliedIndex, 10),
Expand Down Expand Up @@ -590,28 +579,26 @@ func (d *debugRangeData) postProcessing() {
d.Results[debugRangeHeaderKeyRange][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderKeyRange][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease != nil && info.State.Lease != nil {
if leaderStoreInfo.State.Lease.Replica.ReplicaID != info.State.Lease.Replica.ReplicaID {
d.Results[debugRangeHeaderLeaseHolder][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseHolder][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Type() != info.State.Lease.Type() {
d.Results[debugRangeHeaderLeaseType][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseType][info.SourceStoreID].Class = debugRangeClassWarning
}
if d.Results[debugRangeHeaderLeaseEpoch][leaderStoreInfo.SourceStoreID].Value !=
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID].Value {
d.Results[debugRangeHeaderLeaseEpoch][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Start != info.State.Lease.Start {
d.Results[debugRangeHeaderLeaseStart][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseStart][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Expiration != info.State.Lease.Expiration {
d.Results[debugRangeHeaderLeaseExpiration][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseExpiration][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Replica.ReplicaID != info.State.Lease.Replica.ReplicaID {
d.Results[debugRangeHeaderLeaseHolder][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseHolder][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Type() != info.State.Lease.Type() {
d.Results[debugRangeHeaderLeaseType][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseType][info.SourceStoreID].Class = debugRangeClassWarning
}
if d.Results[debugRangeHeaderLeaseEpoch][leaderStoreInfo.SourceStoreID].Value !=
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID].Value {
d.Results[debugRangeHeaderLeaseEpoch][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseEpoch][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Start != info.State.Lease.Start {
d.Results[debugRangeHeaderLeaseStart][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseStart][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.Lease.Expiration != info.State.Lease.Expiration {
d.Results[debugRangeHeaderLeaseExpiration][d.HeaderFakeStoreID].Class = debugRangeClassWarning
d.Results[debugRangeHeaderLeaseExpiration][info.SourceStoreID].Class = debugRangeClassWarning
}
if leaderStoreInfo.State.LeaseAppliedIndex != info.State.LeaseAppliedIndex {
d.Results[debugRangeHeaderLeaseAppliedIndex][d.HeaderFakeStoreID].Class = debugRangeClassWarning
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ func TestRangesResponse(t *testing.T) {
if len(ri.State.Desc.Replicas) != 1 || ri.State.Desc.Replicas[0] != expReplica {
t.Errorf("unexpected replica list %+v", ri.State.Desc.Replicas)
}
if ri.State.Lease == nil {
if ri.State.Lease == (roachpb.Lease{}) {
t.Error("expected a nontrivial Lease")
}
if ri.State.LastIndex == 0 {
Expand Down
18 changes: 9 additions & 9 deletions pkg/storage/client_replica_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func TestRangeTransferLease(t *testing.T) {
}
}

forceLeaseExtension := func(sender *storage.Stores, lease *roachpb.Lease) error {
forceLeaseExtension := func(sender *storage.Stores, lease roachpb.Lease) error {
shouldRenewTS := lease.Expiration.Add(-1, 0)
mtc.manualClock.Set(shouldRenewTS.WallTime + 1)
return sendRead(sender).GoError()
Expand All @@ -588,7 +588,7 @@ func TestRangeTransferLease(t *testing.T) {
t.Fatal(err)
}
newLease, _ := replica0.GetLease()
if err := origLease.Equivalent(*newLease); err != nil {
if err := origLease.Equivalent(newLease); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -1215,7 +1215,7 @@ func TestRangeInfo(t *testing.T) {
expRangeInfos := []roachpb.RangeInfo{
{
Desc: *rhsReplica0.Desc(),
Lease: *rhsLease,
Lease: rhsLease,
},
}
if !reflect.DeepEqual(reply.Header().RangeInfos, expRangeInfos) {
Expand Down Expand Up @@ -1247,11 +1247,11 @@ func TestRangeInfo(t *testing.T) {
expRangeInfos = []roachpb.RangeInfo{
{
Desc: *lhsReplica0.Desc(),
Lease: *lhsLease,
Lease: lhsLease,
},
{
Desc: *rhsReplica0.Desc(),
Lease: *rhsLease,
Lease: rhsLease,
},
}
if !reflect.DeepEqual(reply.Header().RangeInfos, expRangeInfos) {
Expand All @@ -1272,11 +1272,11 @@ func TestRangeInfo(t *testing.T) {
expRangeInfos = []roachpb.RangeInfo{
{
Desc: *rhsReplica0.Desc(),
Lease: *rhsLease,
Lease: rhsLease,
},
{
Desc: *lhsReplica0.Desc(),
Lease: *lhsLease,
Lease: lhsLease,
},
}
if !reflect.DeepEqual(reply.Header().RangeInfos, expRangeInfos) {
Expand All @@ -1303,11 +1303,11 @@ func TestRangeInfo(t *testing.T) {
expRangeInfos = []roachpb.RangeInfo{
{
Desc: *lhsReplica1.Desc(),
Lease: *lhsLease,
Lease: lhsLease,
},
{
Desc: *rhsReplica1.Desc(),
Lease: *rhsLease,
Lease: rhsLease,
},
}
if !reflect.DeepEqual(reply.Header().RangeInfos, expRangeInfos) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func (r *Replica) GetLastIndex() (uint64, error) {
// GetLease exposes replica.getLease for tests.
// If you just need information about the lease holder, consider issuing a
// LeaseInfoRequest instead of using this internal method.
func (r *Replica) GetLease() (*roachpb.Lease, *roachpb.Lease) {
func (r *Replica) GetLease() (roachpb.Lease, *roachpb.Lease) {
return r.getLease()
}

Expand Down
Loading

0 comments on commit c293630

Please sign in to comment.