Skip to content

Commit

Permalink
*: add a few example usages of must
Browse files Browse the repository at this point in the history
Epic: none
Release note: None
  • Loading branch information
erikgrinaker committed Jul 28, 2023
1 parent 5912f89 commit 3d2f8e8
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 77 deletions.
1 change: 1 addition & 0 deletions pkg/kv/kvserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ go_library(
"//pkg/util/metric",
"//pkg/util/metric/aggmetric",
"//pkg/util/mon",
"//pkg/util/must",
"//pkg/util/pprofutil",
"//pkg/util/protoutil",
"//pkg/util/quotapool",
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval",
visibility = ["//visibility:public"],
deps = [
"//pkg/build",
"//pkg/clusterversion",
"//pkg/keys",
"//pkg/kv/kvpb",
Expand Down Expand Up @@ -85,6 +84,7 @@ go_library(
"//pkg/util/limit",
"//pkg/util/log",
"//pkg/util/mon",
"//pkg/util/must",
"//pkg/util/protoutil",
"//pkg/util/tracing",
"//pkg/util/uuid",
Expand Down
90 changes: 43 additions & 47 deletions pkg/kv/kvserver/batcheval/cmd_clear_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/must"
"github.com/cockroachdb/errors"
"github.com/kr/pretty"
)

// ClearRangeBytesThreshold is the threshold over which the ClearRange
Expand Down Expand Up @@ -174,63 +173,60 @@ func computeStatsDelta(

// We can avoid manually computing the stats delta if we're clearing
// the entire range.
entireRange := desc.StartKey.Equal(from) && desc.EndKey.Equal(to)
if entireRange {
if desc.StartKey.Equal(from) && desc.EndKey.Equal(to) {
// Note this it is safe to use the full range MVCC stats, as
// opposed to the usual method of computing only a localizied
// opposed to the usual method of computing only a localized
// stats delta, because a full-range clear prevents any concurrent
// access to the stats. Concurrent changes to range-local keys are
// explicitly ignored (i.e. SysCount, SysBytes).
delta = cArgs.EvalCtx.GetMVCCStats()
delta.SysCount, delta.SysBytes, delta.AbortSpanBytes = 0, 0, 0 // no change to system stats
}

// If we can't use the fast stats path, or race test is enabled, compute stats
// across the key span to be cleared.
if !entireRange || util.RaceEnabled {
computed, err := storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
// Assert correct stats.
if err := must.Expensive(func() error {
if delta.ContainsEstimates != 0 {
return nil
}
computed, err := storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
if err != nil {
return err
}
return must.Equal(ctx, delta, computed, "range MVCC stats differ from computed")
}); err != nil {
return enginepb.MVCCStats{}, err
}

} else {
// If we can't use the fast path, compute stats across the cleared span.
var err error
delta, err = storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
if err != nil {
return enginepb.MVCCStats{}, err
}
// If we took the fast path but race is enabled, assert stats were correctly
// computed.
if entireRange {
// Retain the value of ContainsEstimates for tests under race.
computed.ContainsEstimates = delta.ContainsEstimates
// We only want to assert the correctness of stats that do not contain
// estimates.
if delta.ContainsEstimates == 0 && !delta.Equal(computed) {
log.Fatalf(ctx, "fast-path MVCCStats computation gave wrong result: diff(fast, computed) = %s",
pretty.Diff(delta, computed))
}

// We need to adjust for the fragmentation of any MVCC range tombstones that
// straddle the span bounds. The clearing of the inner fragments has already
// been accounted for above. We take care not to peek outside the Raft range
// bounds.
leftPeekBound, rightPeekBound := rangeTombstonePeekBounds(
from, to, desc.StartKey.AsRawKey(), desc.EndKey.AsRawKey())
rkIter := readWriter.NewMVCCIterator(storage.MVCCKeyIterKind, storage.IterOptions{
KeyTypes: storage.IterKeyTypeRangesOnly,
LowerBound: leftPeekBound,
UpperBound: rightPeekBound,
})
defer rkIter.Close()

if cmp, lhs, err := storage.PeekRangeKeysLeft(rkIter, from); err != nil {
return enginepb.MVCCStats{}, err
} else if cmp > 0 {
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(from, lhs.Versions))
}
delta = computed

// If we're not clearing the entire range, we need to adjust for the
// fragmentation of any MVCC range tombstones that straddle the span bounds.
// The clearing of the inner fragments has already been accounted for above.
// We take care not to peek outside the Raft range bounds.
if !entireRange {
leftPeekBound, rightPeekBound := rangeTombstonePeekBounds(
from, to, desc.StartKey.AsRawKey(), desc.EndKey.AsRawKey())
rkIter := readWriter.NewMVCCIterator(storage.MVCCKeyIterKind, storage.IterOptions{
KeyTypes: storage.IterKeyTypeRangesOnly,
LowerBound: leftPeekBound,
UpperBound: rightPeekBound,
})
defer rkIter.Close()

if cmp, lhs, err := storage.PeekRangeKeysLeft(rkIter, from); err != nil {
return enginepb.MVCCStats{}, err
} else if cmp > 0 {
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(from, lhs.Versions))
}

if cmp, rhs, err := storage.PeekRangeKeysRight(rkIter, to); err != nil {
return enginepb.MVCCStats{}, err
} else if cmp < 0 {
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(to, rhs.Versions))
}
if cmp, rhs, err := storage.PeekRangeKeysRight(rkIter, to); err != nil {
return enginepb.MVCCStats{}, err
} else if cmp < 0 {
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(to, rhs.Versions))
}
}

Expand Down
20 changes: 6 additions & 14 deletions pkg/kv/kvserver/batcheval/cmd_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
Expand All @@ -27,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/must"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/errors"
)
Expand Down Expand Up @@ -274,19 +274,11 @@ func evalExport(
reply.ResumeReason = kvpb.RESUME_ELASTIC_CPU_LIMIT
break
} else {
if !resumeInfo.CPUOverlimit {
// We should never come here. There should be no condition aside from
// resource constraints that results in an early exit without
// exporting any data. Regardless, if we have a resumeKey we
// immediately retry the ExportRequest from that key and timestamp
// onwards.
if !build.IsRelease() {
return result.Result{}, errors.AssertionFailedf("ExportRequest exited without " +
"exporting any data for an unknown reason; programming error")
} else {
log.Warningf(ctx, "unexpected resume span from ExportRequest without exporting any data for an unknown reason: %v", resumeInfo)
}
}
// There should be no condition aside from resource constraints that
// results in an early exit without exporting any data. Regardless, if
// we have a resumeKey we immediately retry the ExportRequest from
// that key and timestamp onwards.
_ = must.True(ctx, resumeInfo.CPUOverlimit, "Export returned no data: %+v", resumeInfo)
start = resumeInfo.ResumeKey.Key
resumeKeyTS = resumeInfo.ResumeKey.Timestamp
continue
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/batcheval/cmd_migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/cockroach/pkg/util/must"
)

func init() {
Expand Down Expand Up @@ -63,8 +63,8 @@ func Migrate(
migrationVersion := args.Version

fn, ok := migrationRegistry[migrationVersion]
if !ok {
return result.Result{}, errors.AssertionFailedf("migration for %s not found", migrationVersion)
if err := must.True(ctx, ok, "migration for %s not found", migrationVersion); err != nil {
return result.Result{}, err
}
pd, err := fn(ctx, readWriter, cArgs)
if err != nil {
Expand Down
10 changes: 4 additions & 6 deletions pkg/kv/kvserver/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"unsafe"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/must"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -400,11 +400,9 @@ func (ss *raftSchedulerShard) worker(
state.flags |= stateRaftReady
}
}
if util.RaceEnabled { // assert the ticks invariant
if tick := state.flags&stateRaftTick != 0; tick != (state.ticks != 0) {
log.Fatalf(ctx, "stateRaftTick is %v with ticks %v", tick, state.ticks)
}
}

_ = must.Equal(ctx, state.flags&stateRaftTick != 0, state.ticks != 0,
"flags %d with %d ticks", state.flags, state.ticks) // safe to continue
if state.flags&stateRaftTick != 0 {
for t := state.ticks; t > 0; t-- {
// processRaftTick returns true if the range should perform ready
Expand Down
1 change: 1 addition & 0 deletions pkg/roachpb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ go_library(
"//pkg/util/humanizeutil",
"//pkg/util/interval",
"//pkg/util/log",
"//pkg/util/must",
"//pkg/util/protoutil",
"//pkg/util/syncutil",
"//pkg/util/timetz",
Expand Down
10 changes: 4 additions & 6 deletions pkg/roachpb/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/bitarray"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/encoding"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/interval"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/must"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/timetz"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
Expand Down Expand Up @@ -2464,11 +2464,9 @@ var _ sort.Interface = SequencedWriteBySeq{}
// Find searches for the index of the SequencedWrite with the provided
// sequence number. Returns -1 if no corresponding write is found.
func (s SequencedWriteBySeq) Find(seq enginepb.TxnSeq) int {
if util.RaceEnabled {
if !sort.IsSorted(s) {
panic("SequencedWriteBySeq must be sorted")
}
}
_ = must.Expensive(func() error {
return must.True(context.TODO(), sort.IsSorted(s), "SequencedWriteBySeq not sorted")
})
if i := sort.Search(len(s), func(i int) bool {
return s[i].Sequence >= seq
}); i < len(s) && s[i].Sequence == seq {
Expand Down

0 comments on commit 3d2f8e8

Please sign in to comment.