Skip to content

Commit

Permalink
kvserver: delete generation_compatible
Browse files Browse the repository at this point in the history
Range descriptors have gotten generations in 19.1. Their start was
rocky: for a little while before 19.1 was released the code for setting
the generations was silly and overlapping descriptors didn't have the
property that the one with the higher generation is more recent. The
code then got better for the 19.1 final release, but in order to support
snapshots created by bad code being applied by good code, the extra
generation_compatible field was introduced. See #36654.

Years have passed and we don't have to worry about snapshots created by
pre-19.1 code being applied any more. This patch removes a bunch of
noise generated by that field.

Release note: None
  • Loading branch information
andreimatei committed May 15, 2020
1 parent 7499244 commit 9b449c5
Show file tree
Hide file tree
Showing 16 changed files with 230 additions and 441 deletions.
47 changes: 9 additions & 38 deletions c-deps/libroach/protos/roachpb/metadata.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 0 additions & 34 deletions c-deps/libroach/protos/roachpb/metadata.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/cli/cli_debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Example_debug_decode_key_value() {
// if the result below looks like garbage, then it likely is:
//
// 0.987654321,0 /Local/Range/Table/53/1/-4560243296450227838/RangeDescriptor (0x016b12bd8980c0b6c2e211ba518200017264736300000000003ade68b109): [/Table/53/1/-4560243296450227838, /Table/53/1/-4559358311118345834)
// Raw:r1179:/Table/53/1/-45{60243296450227838-59358311118345834} [(n1,s1):1, (n4,s4):2, (n2,s2):4, next=5, gen=4?]
// Raw:r1179:/Table/53/1/-45{60243296450227838-59358311118345834} [(n1,s1):1, (n4,s4):2, (n2,s2):4, next=5, gen=4]
}

func TestDebugKeysHex(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions pkg/kv/kvclient/kvcoord/dist_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3416,6 +3416,7 @@ func TestEvictionTokenCoalesce(t *testing.T) {
stopper := stop.NewStopper()
defer stopper.Stop(context.TODO())

initGen := int64(1)
testUserRangeDescriptor := roachpb.RangeDescriptor{
RangeID: 2,
StartKey: roachpb.RKey("a"),
Expand All @@ -3426,6 +3427,7 @@ func TestEvictionTokenCoalesce(t *testing.T) {
StoreID: 1,
},
},
Generation: initGen,
}

clock := hlc.NewClock(hlc.UnixNano, time.Nanosecond)
Expand Down Expand Up @@ -3491,7 +3493,7 @@ func TestEvictionTokenCoalesce(t *testing.T) {
if err := testutils.SucceedsSoonError(func() error {
// Since the previously fetched RangeDescriptor was ["a", "d"), the request keys
// would be coalesced to "a".
numCalls := ds.rangeCache.lookupRequests.NumCalls(string(roachpb.RKey("a")) + ":false")
numCalls := ds.rangeCache.lookupRequests.NumCalls(fmt.Sprintf("a:false:%d", initGen))
if numCalls != 2 {
return errors.Errorf("expected %d in-flight requests, got %d", 2, numCalls)
}
Expand Down Expand Up @@ -3543,7 +3545,7 @@ func TestDistSenderSlowLogMessage(t *testing.T) {
desc := &roachpb.RangeDescriptor{RangeID: 9, StartKey: roachpb.RKey("x")}
{
exp := `have been waiting 8.16s (120 attempts) for RPC to` +
` r9:{-} [<no replicas>, next=0, gen=0?]: boom`
` r9:{-} [<no replicas>, next=0, gen=0]: boom`
act := slowRangeRPCWarningStr(
dur,
120,
Expand Down
46 changes: 11 additions & 35 deletions pkg/kv/kvclient/kvcoord/range_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func makeLookupRequestKey(
// split again into [a, b) and [b, c), we don't want to the requests on [a,
// b) to be coalesced with the retried requests on [c, e). To distinguish the
// two cases, we can use the generation of the previous descriptor.
if prevDesc != nil && prevDesc.GetGenerationComparable() {
if prevDesc != nil {
ret.WriteString(":")
ret.WriteString(strconv.FormatInt(prevDesc.Generation, 10))
}
Expand Down Expand Up @@ -540,23 +540,12 @@ func (rdc *RangeDescriptorCache) evictCachedRangeDescriptorLocked(

// Note that we're doing a "compare-and-erase": If seenDesc is not nil, we
// want to clean the cache only if it equals the cached range descriptor. We
// try to use Generation and GenerationComparable to determine if the range
// descriptors are equal, but if we cannot, we fallback to
// pointer-comparison. If the range descriptors are not equal, then likely
// some other caller already evicted previously, and we can save work by not
// doing it again (which would prompt another expensive lookup).
// use Generation to determine if the range descriptors are equal. If the
// range descriptors are not equal, then likely some other caller already
// evicted previously, and we can save work by not doing it again (which would
// prompt another expensive lookup).
if seenDesc != nil {
if seenDesc.GetGenerationComparable() && cachedDesc.GetGenerationComparable() {
if seenDesc.Generation != cachedDesc.Generation {
return nil
}
} else if !seenDesc.GetGenerationComparable() && !cachedDesc.GetGenerationComparable() {
if seenDesc != cachedDesc {
return nil
}
} else {
// One descriptor's generation is comparable, while the other is
// incomparable, so the descriptors are guaranteed to be different.
if seenDesc.Generation != cachedDesc.Generation {
return nil
}
}
Expand Down Expand Up @@ -683,15 +672,8 @@ func (rdc *RangeDescriptorCache) clearOverlappingCachedRangeDescriptors(
// check ["c", "d"). We do, however, want to check ["b", "c"), which is why
// the end key is inclusive.
if cached.StartKey.Less(desc.EndKey) && !cached.EndKey.Less(desc.EndKey) {
if desc.GetGenerationComparable() && cached.GetGenerationComparable() {
if desc.Generation <= cached.Generation {
// Generations are comparable and a newer descriptor already exists in
// cache.
continueWithInsert = false
}
} else if desc.Equal(*cached) {
// Generations are incomparable so we don't continue with insertion
// only if the descriptor already exists.
if desc.Generation <= cached.Generation {
// A newer descriptor already exists in cache.
continueWithInsert = false
}
if continueWithInsert {
Expand All @@ -715,16 +697,10 @@ func (rdc *RangeDescriptorCache) clearOverlappingCachedRangeDescriptors(
// cache unconditionally.
rdc.rangeCache.cache.DoRangeEntry(func(e *cache.Entry) bool {
descriptor := e.Value.(*roachpb.RangeDescriptor)
if desc.GetGenerationComparable() && descriptor.GetGenerationComparable() {
// If generations are comparable, then check generations to see if we
// evict.
if desc.Generation <= descriptor.Generation {
continueWithInsert = false
} else {
entriesToEvict = append(entriesToEvict, e)
}
// Check generations to see if we evict.
if desc.Generation <= descriptor.Generation {
continueWithInsert = false
} else {
// If generations are not comparable, evict.
entriesToEvict = append(entriesToEvict, e)
}
return false
Expand Down
Loading

0 comments on commit 9b449c5

Please sign in to comment.