Skip to content

Commit 09fb2a8

Browse files
committed
kvclient: remove unnecessary getters
Now that the RangeCache.CacheEntry is private, we no longer need all the getters for the different fields. This simplifies some test code that was converting back and froth from pointers, structs. Epic: none Release note: None
1 parent dd17998 commit 09fb2a8

File tree

2 files changed

+106
-150
lines changed

2 files changed

+106
-150
lines changed

pkg/kv/kvclient/rangecache/range_cache.go

Lines changed: 30 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ func (et *EvictionToken) syncRLocked(
377377
ctx context.Context,
378378
) (stillValid bool, cachedEntry *cacheEntry, rawEntry *cache.Entry) {
379379
cachedEntry, rawEntry = et.rdc.getCachedRLocked(ctx, et.entry.desc.StartKey, false /* inverted */)
380-
if cachedEntry == nil || !descsCompatible(cachedEntry.Desc(), et.Desc()) {
380+
if cachedEntry == nil || !descsCompatible(&cachedEntry.desc, et.Desc()) {
381381
et.clear()
382382
return false, nil, nil
383383
}
@@ -438,7 +438,7 @@ func (et *EvictionToken) SyncTokenAndMaybeUpdateCache(
438438
ri := roachpb.RangeInfo{
439439
Desc: *rangeDesc,
440440
Lease: *l,
441-
ClosedTimestampPolicy: et.entry.ClosedTimestampPolicy(),
441+
ClosedTimestampPolicy: et.entry.closedts,
442442
}
443443
et.evictAndReplaceLocked(ctx, ri)
444444
return false
@@ -605,7 +605,7 @@ func (rc *RangeCache) LookupRangeID(
605605
if err != nil {
606606
return 0, err
607607
}
608-
return tok.entry.Desc().RangeID, nil
608+
return tok.entry.desc.RangeID, nil
609609
}
610610

611611
// Lookup presents a simpler interface for looking up a RangeDescriptor for a
@@ -640,7 +640,7 @@ func (rc *RangeCache) getCachedOverlappingRLocked(
640640
defer from.release()
641641
var res []*cache.Entry
642642
rc.rangeCache.cache.DoRangeReverseEntry(func(e *cache.Entry) (exit bool) {
643-
desc := rc.getValue(e).Desc()
643+
desc := rc.getValue(e).desc
644644
if desc.StartKey.Equal(span.EndKey) {
645645
// Skip over descriptor starting at the end key, who'd supposed to be exclusive.
646646
return false
@@ -1013,7 +1013,7 @@ func (rc *RangeCache) evictDescLocked(ctx context.Context, desc *roachpb.RangeDe
10131013
// Cache is empty; nothing to do.
10141014
return false
10151015
}
1016-
cachedDesc := cachedEntry.Desc()
1016+
cachedDesc := cachedEntry.desc
10171017
cachedNewer := cachedDesc.Generation > desc.Generation
10181018
if cachedNewer {
10191019
return false
@@ -1096,7 +1096,7 @@ func (rc *RangeCache) getCachedRLocked(
10961096
}
10971097

10981098
// Return nil if the key does not belong to the range.
1099-
if !containsFn(entry.Desc(), key) {
1099+
if !containsFn(&entry.desc, key) {
11001100
return nil, nil
11011101
}
11021102
return entry, rawEntry
@@ -1142,7 +1142,7 @@ func (rc *RangeCache) insertLockedInner(ctx context.Context, rs []*cacheEntry) [
11421142
_, ok := ent.desc.GetReplicaDescriptorByID(replID)
11431143
if !ok {
11441144
log.Fatalf(ctx, "leaseholder replicaID: %d not part of descriptor: %s. lease: %s",
1145-
replID, ent.Desc(), ent.Lease())
1145+
replID, ent.desc, ent.lease)
11461146
}
11471147
}
11481148
// Note: we append the end key of each range to meta records
@@ -1195,10 +1195,10 @@ func (rc *RangeCache) clearOlderOverlapping(
11951195
func (rc *RangeCache) clearOlderOverlappingLocked(
11961196
ctx context.Context, newEntry *cacheEntry,
11971197
) (ok bool, newerEntry *cacheEntry) {
1198-
log.VEventf(ctx, 2, "clearing entries overlapping %s", newEntry.Desc())
1198+
log.VEventf(ctx, 2, "clearing entries overlapping %s", newEntry.desc)
11991199
newest := true
12001200
var newerFound *cacheEntry
1201-
overlapping := rc.getCachedOverlappingRLocked(ctx, newEntry.Desc().RSpan())
1201+
overlapping := rc.getCachedOverlappingRLocked(ctx, newEntry.desc.RSpan())
12021202
for _, e := range overlapping {
12031203
entry := rc.getValue(e)
12041204
if newEntry.overrides(entry) {
@@ -1208,7 +1208,7 @@ func (rc *RangeCache) clearOlderOverlappingLocked(
12081208
rc.delEntryLocked(e)
12091209
} else {
12101210
newest = false
1211-
if descsCompatible(entry.Desc(), newEntry.Desc()) {
1211+
if descsCompatible(&entry.desc, &newEntry.desc) {
12121212
newerFound = entry
12131213
// We've found a similar descriptor in the cache; there can't be any
12141214
// other overlapping ones so let's stop the iteration.
@@ -1230,7 +1230,7 @@ func (rc *RangeCache) swapEntryLocked(
12301230
) {
12311231
if newEntry != nil {
12321232
old := rc.getValue(oldEntry)
1233-
if !descsCompatible(old.Desc(), newEntry.Desc()) {
1233+
if !descsCompatible(&old.desc, &newEntry.desc) {
12341234
log.Fatalf(ctx, "attempting to swap non-compatible descs: %s vs %s",
12351235
old, newEntry)
12361236
}
@@ -1244,7 +1244,7 @@ func (rc *RangeCache) swapEntryLocked(
12441244
}
12451245

12461246
func (rc *RangeCache) addEntryLocked(entry *cacheEntry) {
1247-
key := newRangeCacheKey(entry.Desc().StartKey)
1247+
key := newRangeCacheKey(entry.desc.StartKey)
12481248
rc.rangeCache.cache.Add(key, entry)
12491249
}
12501250

@@ -1300,45 +1300,6 @@ type cacheEntry struct {
13001300
closedts roachpb.RangeClosedTimestampPolicy
13011301
}
13021302

1303-
func (e cacheEntry) String() string {
1304-
return fmt.Sprintf("desc:%s, lease:%s", e.Desc(), e.lease)
1305-
}
1306-
1307-
// Desc returns the cached descriptor. Note that, besides being possibly stale,
1308-
// this descriptor also might not represent a descriptor that was ever
1309-
// committed. See DescSpeculative().
1310-
func (e *cacheEntry) Desc() *roachpb.RangeDescriptor {
1311-
return &e.desc
1312-
}
1313-
1314-
// Leaseholder returns the cached leaseholder replica, if known. Returns nil if
1315-
// the leaseholder is not known.
1316-
func (e *cacheEntry) Leaseholder() *roachpb.ReplicaDescriptor {
1317-
if e.lease.Empty() {
1318-
return nil
1319-
}
1320-
return &e.lease.Replica
1321-
}
1322-
1323-
// Lease returns the cached lease, if known. Returns nil if no lease is known.
1324-
// It's possible for a leaseholder to be known, but not a full lease, in which
1325-
// case Leaseholder() returns non-nil but Lease() returns nil.
1326-
func (e *cacheEntry) Lease() *roachpb.Lease {
1327-
if e.lease.Empty() {
1328-
return nil
1329-
}
1330-
if e.LeaseSpeculative() {
1331-
return nil
1332-
}
1333-
return &e.lease
1334-
}
1335-
1336-
// ClosedTimestampPolicy returns the cached understanding of the range's closed
1337-
// timestamp policy. If no policy is known, LAG_BY_CLUSTER_SETTING is returned.
1338-
func (e *cacheEntry) ClosedTimestampPolicy() roachpb.RangeClosedTimestampPolicy {
1339-
return e.closedts
1340-
}
1341-
13421303
// DescSpeculative returns true if the descriptor in the entry is "speculative"
13431304
// - i.e. it doesn't correspond to a committed value. Such descriptors have been
13441305
// inserted in the cache with Generation=0.
@@ -1358,11 +1319,15 @@ func (e *cacheEntry) LeaseSpeculative() bool {
13581319
return e.lease.Speculative()
13591320
}
13601321

1322+
func (e cacheEntry) String() string {
1323+
return fmt.Sprintf("desc:%s, lease:%s", e.desc, e.lease)
1324+
}
1325+
13611326
func (e *cacheEntry) toRangeInfo() roachpb.RangeInfo {
13621327
return roachpb.RangeInfo{
13631328
Desc: e.desc,
13641329
Lease: e.lease,
1365-
ClosedTimestampPolicy: e.ClosedTimestampPolicy(),
1330+
ClosedTimestampPolicy: e.closedts,
13661331
}
13671332
}
13681333

@@ -1381,8 +1346,8 @@ func (e *cacheEntry) toRangeInfo() roachpb.RangeInfo {
13811346
// "speculative" (sequence=0).
13821347
func (e *cacheEntry) overrides(o *cacheEntry) bool {
13831348
if util.RaceEnabled {
1384-
if _, err := e.Desc().RSpan().Intersect(o.Desc().RSpan()); err != nil {
1385-
panic(fmt.Sprintf("descriptors don't intersect: %s vs %s", e.Desc(), o.Desc()))
1349+
if _, err := e.desc.RSpan().Intersect(o.desc.RSpan()); err != nil {
1350+
panic(fmt.Sprintf("descriptors don't intersect: %s vs %s", e.desc, o.desc))
13861351
}
13871352
}
13881353

@@ -1395,9 +1360,9 @@ func (e *cacheEntry) overrides(o *cacheEntry) bool {
13951360
// If two RangeDescriptors overlap and have the same Generation, they must
13961361
// be referencing the same range, in which case their lease sequences are
13971362
// comparable.
1398-
if e.Desc().RangeID != o.Desc().RangeID {
1363+
if e.desc.RangeID != o.desc.RangeID {
13991364
panic(fmt.Sprintf("overlapping descriptors with same gen but different IDs: %s vs %s",
1400-
e.Desc(), o.Desc()))
1365+
e.desc, o.desc))
14011366
}
14021367

14031368
if res := compareEntryLeases(o, e); res != 0 {
@@ -1421,8 +1386,8 @@ func (e *cacheEntry) overrides(o *cacheEntry) bool {
14211386
// older; this matches the semantics of b.overrides(a).
14221387
func compareEntryDescs(a, b *cacheEntry) int {
14231388
if util.RaceEnabled {
1424-
if _, err := a.Desc().RSpan().Intersect(b.Desc().RSpan()); err != nil {
1425-
panic(fmt.Sprintf("descriptors don't intersect: %s vs %s", a.Desc(), b.Desc()))
1389+
if _, err := a.desc.RSpan().Intersect(b.desc.RSpan()); err != nil {
1390+
panic(fmt.Sprintf("descriptors don't intersect: %s vs %s", a.desc, b.desc))
14261391
}
14271392
}
14281393

@@ -1434,10 +1399,10 @@ func compareEntryDescs(a, b *cacheEntry) int {
14341399
return -1
14351400
}
14361401

1437-
if a.Desc().Generation < b.Desc().Generation {
1402+
if a.desc.Generation < b.desc.Generation {
14381403
return -1
14391404
}
1440-
if a.Desc().Generation > b.Desc().Generation {
1405+
if a.desc.Generation > b.desc.Generation {
14411406
return 1
14421407
}
14431408
return 0
@@ -1467,10 +1432,10 @@ func compareEntryLeases(a, b *cacheEntry) int {
14671432
return -1
14681433
}
14691434

1470-
if a.Lease().Sequence < b.Lease().Sequence {
1435+
if a.lease.Sequence < b.lease.Sequence {
14711436
return -1
14721437
}
1473-
if a.Lease().Sequence > b.Lease().Sequence {
1438+
if a.lease.Sequence > b.lease.Sequence {
14741439
return 1
14751440
}
14761441
return 0
@@ -1502,9 +1467,9 @@ func compareEntryLeases(a, b *cacheEntry) int {
15021467
func (e *cacheEntry) maybeUpdate(
15031468
ctx context.Context, l *roachpb.Lease, rangeDesc *roachpb.RangeDescriptor,
15041469
) (updated, updatedLease bool, newEntry *cacheEntry) {
1505-
if !descsCompatible(e.Desc(), rangeDesc) {
1470+
if !descsCompatible(&e.desc, rangeDesc) {
15061471
log.Fatalf(ctx, "attempting to update by comparing non-compatible descs: %s vs %s",
1507-
e.Desc(), rangeDesc)
1472+
e.desc, rangeDesc)
15081473
}
15091474

15101475
newEntry = &cacheEntry{
@@ -1562,7 +1527,7 @@ func (e *cacheEntry) maybeUpdate(
15621527
2,
15631528
"incompatible leaseholder id: %d/descriptor %v pair; eliding lease update to the cache",
15641529
newEntry.lease.Replica.ReplicaID,
1565-
newEntry.Desc(),
1530+
newEntry.desc,
15661531
)
15671532
newEntry.lease = roachpb.Lease{}
15681533
updatedLease = false

0 commit comments

Comments
 (0)