Skip to content

Commit 0a1699d

Browse files
committed
*: add a few example usages of must
Epic: none Release note: None
1 parent 3250477 commit 0a1699d

File tree

8 files changed

+63
-77
lines changed

8 files changed

+63
-77
lines changed

pkg/kv/kvserver/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ go_library(
206206
"//pkg/util/metric",
207207
"//pkg/util/metric/aggmetric",
208208
"//pkg/util/mon",
209+
"//pkg/util/must",
209210
"//pkg/util/pprofutil",
210211
"//pkg/util/protoutil",
211212
"//pkg/util/quotapool",

pkg/kv/kvserver/batcheval/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ go_library(
5555
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval",
5656
visibility = ["//visibility:public"],
5757
deps = [
58-
"//pkg/build",
5958
"//pkg/clusterversion",
6059
"//pkg/keys",
6160
"//pkg/kv/kvpb",
@@ -85,6 +84,7 @@ go_library(
8584
"//pkg/util/limit",
8685
"//pkg/util/log",
8786
"//pkg/util/mon",
87+
"//pkg/util/must",
8888
"//pkg/util/protoutil",
8989
"//pkg/util/tracing",
9090
"//pkg/util/uuid",

pkg/kv/kvserver/batcheval/cmd_clear_range.go

Lines changed: 43 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,9 @@ import (
2323
"github.com/cockroachdb/cockroach/pkg/roachpb"
2424
"github.com/cockroachdb/cockroach/pkg/storage"
2525
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
26-
"github.com/cockroachdb/cockroach/pkg/util"
2726
"github.com/cockroachdb/cockroach/pkg/util/log"
27+
"github.com/cockroachdb/cockroach/pkg/util/must"
2828
"github.com/cockroachdb/errors"
29-
"github.com/kr/pretty"
3029
)
3130

3231
// ClearRangeBytesThreshold is the threshold over which the ClearRange
@@ -174,63 +173,60 @@ func computeStatsDelta(
174173

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

188-
// If we can't use the fast stats path, or race test is enabled, compute stats
189-
// across the key span to be cleared.
190-
if !entireRange || util.RaceEnabled {
191-
computed, err := storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
185+
// Assert correct stats.
186+
if err := must.Expensive(func() error {
187+
if delta.ContainsEstimates != 0 {
188+
return nil
189+
}
190+
computed, err := storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
191+
if err != nil {
192+
return err
193+
}
194+
return must.Equal(ctx, delta, computed, "range MVCC stats differ from computed")
195+
}); err != nil {
196+
return enginepb.MVCCStats{}, err
197+
}
198+
199+
} else {
200+
// If we can't use the fast path, compute stats across the cleared span.
201+
var err error
202+
delta, err = storage.ComputeStats(readWriter, from, to, delta.LastUpdateNanos)
192203
if err != nil {
193204
return enginepb.MVCCStats{}, err
194205
}
195-
// If we took the fast path but race is enabled, assert stats were correctly
196-
// computed.
197-
if entireRange {
198-
// Retain the value of ContainsEstimates for tests under race.
199-
computed.ContainsEstimates = delta.ContainsEstimates
200-
// We only want to assert the correctness of stats that do not contain
201-
// estimates.
202-
if delta.ContainsEstimates == 0 && !delta.Equal(computed) {
203-
log.Fatalf(ctx, "fast-path MVCCStats computation gave wrong result: diff(fast, computed) = %s",
204-
pretty.Diff(delta, computed))
205-
}
206+
207+
// We need to adjust for the fragmentation of any MVCC range tombstones that
208+
// straddle the span bounds. The clearing of the inner fragments has already
209+
// been accounted for above. We take care not to peek outside the Raft range
210+
// bounds.
211+
leftPeekBound, rightPeekBound := rangeTombstonePeekBounds(
212+
from, to, desc.StartKey.AsRawKey(), desc.EndKey.AsRawKey())
213+
rkIter := readWriter.NewMVCCIterator(storage.MVCCKeyIterKind, storage.IterOptions{
214+
KeyTypes: storage.IterKeyTypeRangesOnly,
215+
LowerBound: leftPeekBound,
216+
UpperBound: rightPeekBound,
217+
})
218+
defer rkIter.Close()
219+
220+
if cmp, lhs, err := storage.PeekRangeKeysLeft(rkIter, from); err != nil {
221+
return enginepb.MVCCStats{}, err
222+
} else if cmp > 0 {
223+
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(from, lhs.Versions))
206224
}
207-
delta = computed
208-
209-
// If we're not clearing the entire range, we need to adjust for the
210-
// fragmentation of any MVCC range tombstones that straddle the span bounds.
211-
// The clearing of the inner fragments has already been accounted for above.
212-
// We take care not to peek outside the Raft range bounds.
213-
if !entireRange {
214-
leftPeekBound, rightPeekBound := rangeTombstonePeekBounds(
215-
from, to, desc.StartKey.AsRawKey(), desc.EndKey.AsRawKey())
216-
rkIter := readWriter.NewMVCCIterator(storage.MVCCKeyIterKind, storage.IterOptions{
217-
KeyTypes: storage.IterKeyTypeRangesOnly,
218-
LowerBound: leftPeekBound,
219-
UpperBound: rightPeekBound,
220-
})
221-
defer rkIter.Close()
222-
223-
if cmp, lhs, err := storage.PeekRangeKeysLeft(rkIter, from); err != nil {
224-
return enginepb.MVCCStats{}, err
225-
} else if cmp > 0 {
226-
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(from, lhs.Versions))
227-
}
228225

229-
if cmp, rhs, err := storage.PeekRangeKeysRight(rkIter, to); err != nil {
230-
return enginepb.MVCCStats{}, err
231-
} else if cmp < 0 {
232-
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(to, rhs.Versions))
233-
}
226+
if cmp, rhs, err := storage.PeekRangeKeysRight(rkIter, to); err != nil {
227+
return enginepb.MVCCStats{}, err
228+
} else if cmp < 0 {
229+
delta.Subtract(storage.UpdateStatsOnRangeKeySplit(to, rhs.Versions))
234230
}
235231
}
236232

pkg/kv/kvserver/batcheval/cmd_export.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"fmt"
1717
"time"
1818

19-
"github.com/cockroachdb/cockroach/pkg/build"
2019
"github.com/cockroachdb/cockroach/pkg/keys"
2120
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
2221
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
@@ -27,6 +26,7 @@ import (
2726
"github.com/cockroachdb/cockroach/pkg/storage"
2827
"github.com/cockroachdb/cockroach/pkg/util/hlc"
2928
"github.com/cockroachdb/cockroach/pkg/util/log"
29+
"github.com/cockroachdb/cockroach/pkg/util/must"
3030
"github.com/cockroachdb/cockroach/pkg/util/tracing"
3131
"github.com/cockroachdb/errors"
3232
)
@@ -274,19 +274,11 @@ func evalExport(
274274
reply.ResumeReason = kvpb.RESUME_ELASTIC_CPU_LIMIT
275275
break
276276
} else {
277-
if !resumeInfo.CPUOverlimit {
278-
// We should never come here. There should be no condition aside from
279-
// resource constraints that results in an early exit without
280-
// exporting any data. Regardless, if we have a resumeKey we
281-
// immediately retry the ExportRequest from that key and timestamp
282-
// onwards.
283-
if !build.IsRelease() {
284-
return result.Result{}, errors.AssertionFailedf("ExportRequest exited without " +
285-
"exporting any data for an unknown reason; programming error")
286-
} else {
287-
log.Warningf(ctx, "unexpected resume span from ExportRequest without exporting any data for an unknown reason: %v", resumeInfo)
288-
}
289-
}
277+
// There should be no condition aside from resource constraints that
278+
// results in an early exit without exporting any data. Regardless, if
279+
// we have a resumeKey we immediately retry the ExportRequest from
280+
// that key and timestamp onwards.
281+
_ = must.True(ctx, resumeInfo.CPUOverlimit, "Export returned no data: %+v", resumeInfo)
290282
start = resumeInfo.ResumeKey.Key
291283
resumeKeyTS = resumeInfo.ResumeKey.Timestamp
292284
continue

pkg/kv/kvserver/batcheval/cmd_migrate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
2323
"github.com/cockroachdb/cockroach/pkg/roachpb"
2424
"github.com/cockroachdb/cockroach/pkg/storage"
25-
"github.com/cockroachdb/errors"
25+
"github.com/cockroachdb/cockroach/pkg/util/must"
2626
)
2727

2828
func init() {
@@ -63,8 +63,8 @@ func Migrate(
6363
migrationVersion := args.Version
6464

6565
fn, ok := migrationRegistry[migrationVersion]
66-
if !ok {
67-
return result.Result{}, errors.AssertionFailedf("migration for %s not found", migrationVersion)
66+
if err := must.True(ctx, ok, "migration for %s not found", migrationVersion); err != nil {
67+
return result.Result{}, err
6868
}
6969
pd, err := fn(ctx, readWriter, cArgs)
7070
if err != nil {

pkg/kv/kvserver/scheduler.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import (
1818
"unsafe"
1919

2020
"github.com/cockroachdb/cockroach/pkg/roachpb"
21-
"github.com/cockroachdb/cockroach/pkg/util"
2221
"github.com/cockroachdb/cockroach/pkg/util/log"
22+
"github.com/cockroachdb/cockroach/pkg/util/must"
2323
"github.com/cockroachdb/cockroach/pkg/util/stop"
2424
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
2525
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -400,11 +400,9 @@ func (ss *raftSchedulerShard) worker(
400400
state.flags |= stateRaftReady
401401
}
402402
}
403-
if util.RaceEnabled { // assert the ticks invariant
404-
if tick := state.flags&stateRaftTick != 0; tick != (state.ticks != 0) {
405-
log.Fatalf(ctx, "stateRaftTick is %v with ticks %v", tick, state.ticks)
406-
}
407-
}
403+
404+
_ = must.Equal(ctx, state.flags&stateRaftTick != 0, state.ticks != 0,
405+
"flags %d with %d ticks", state.flags, state.ticks) // safe to continue
408406
if state.flags&stateRaftTick != 0 {
409407
for t := state.ticks; t > 0; t-- {
410408
// processRaftTick returns true if the range should perform ready

pkg/roachpb/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ go_library(
3838
"//pkg/util/humanizeutil",
3939
"//pkg/util/interval",
4040
"//pkg/util/log",
41+
"//pkg/util/must",
4142
"//pkg/util/protoutil",
4243
"//pkg/util/syncutil",
4344
"//pkg/util/timetz",

pkg/roachpb/data.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ import (
3232
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
3333
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
3434
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
35-
"github.com/cockroachdb/cockroach/pkg/util"
3635
"github.com/cockroachdb/cockroach/pkg/util/bitarray"
3736
"github.com/cockroachdb/cockroach/pkg/util/duration"
3837
"github.com/cockroachdb/cockroach/pkg/util/encoding"
3938
"github.com/cockroachdb/cockroach/pkg/util/hlc"
4039
"github.com/cockroachdb/cockroach/pkg/util/interval"
4140
"github.com/cockroachdb/cockroach/pkg/util/log"
41+
"github.com/cockroachdb/cockroach/pkg/util/must"
4242
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
4343
"github.com/cockroachdb/cockroach/pkg/util/timetz"
4444
"github.com/cockroachdb/cockroach/pkg/util/uuid"
@@ -2464,11 +2464,9 @@ var _ sort.Interface = SequencedWriteBySeq{}
24642464
// Find searches for the index of the SequencedWrite with the provided
24652465
// sequence number. Returns -1 if no corresponding write is found.
24662466
func (s SequencedWriteBySeq) Find(seq enginepb.TxnSeq) int {
2467-
if util.RaceEnabled {
2468-
if !sort.IsSorted(s) {
2469-
panic("SequencedWriteBySeq must be sorted")
2470-
}
2471-
}
2467+
_ = must.Expensive(func() error {
2468+
return must.True(context.TODO(), sort.IsSorted(s), "SequencedWriteBySeq not sorted")
2469+
})
24722470
if i := sort.Search(len(s), func(i int) bool {
24732471
return s[i].Sequence >= seq
24742472
}); i < len(s) && s[i].Sequence == seq {

0 commit comments

Comments
 (0)