forked from cockroachdb/cockroach
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
kvconnectorccl: allow secondary tenants to prefetch range lookups
This patch permits the tenant connector to request more than 0 ranges to be prefetched. In order to enable this, we add logic in the implementation of the RangeLookup RPC to filter any results which are not intended for this tenant. Fixes cockroachdb#91433 Release note: None
- Loading branch information
Showing
8 changed files
with
247 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| // Copyright 2022 The Cockroach Authors. | ||
| // | ||
| // Licensed as a CockroachDB Enterprise file under the Cockroach Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt | ||
|
|
||
| package kvtenantccl_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/base" | ||
| "github.com/cockroachdb/cockroach/pkg/keys" | ||
| "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" | ||
| "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangecache" | ||
| "github.com/cockroachdb/cockroach/pkg/kv/kvserver" | ||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestRangeLookupPrefetchFiltering is an integration test to ensure that | ||
| // range results are filtered for the client. | ||
| func TestRangeLookupPrefetchFiltering(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
|
|
||
| ctx := context.Background() | ||
| tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ | ||
| ServerArgs: base.TestServerArgs{ | ||
| DisableDefaultTestTenant: true, // we're going to manually add tenants | ||
| Knobs: base.TestingKnobs{ | ||
| Store: &kvserver.StoreTestingKnobs{ | ||
| DisableMergeQueue: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| defer tc.Stopper().Stop(ctx) | ||
|
|
||
| ten2ID := roachpb.MustMakeTenantID(2) | ||
| tenant2, err := tc.Server(0).StartTenant(ctx, base.TestTenantArgs{ | ||
| TenantID: ten2ID, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // Split some ranges within tenant2 that we'll want to see in prefetch. | ||
| ten2Codec := keys.MakeSQLCodec(ten2ID) | ||
| ten2Split1 := append(ten2Codec.TenantPrefix(), 'a') | ||
| ten2Split2 := append(ten2Codec.TenantPrefix(), 'b') | ||
| { | ||
| tc.SplitRangeOrFatal(t, ten2Split1) | ||
| tc.SplitRangeOrFatal(t, ten2Split2) | ||
| } | ||
|
|
||
| // Split some ranges for the tenant which comes after tenant2. | ||
| { | ||
| ten3Codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(3)) | ||
| tc.SplitRangeOrFatal(t, ten3Codec.TenantPrefix()) | ||
| tc.SplitRangeOrFatal(t, append(ten3Codec.TenantPrefix(), 'b')) | ||
| tc.SplitRangeOrFatal(t, append(ten3Codec.TenantPrefix(), 'c')) | ||
| } | ||
|
|
||
| // Do the fetch and make sure we prefetch all the ranges we should see, | ||
| // and none of the ranges we should not. | ||
| db := tenant2.DistSenderI().(*kvcoord.DistSender).RangeDescriptorCache().DB() | ||
| prefixRKey := keys.MustAddr(ten2Codec.TenantPrefix()) | ||
| res, prefetch, err := db.RangeLookup( | ||
| ctx, prefixRKey, | ||
| rangecache.ReadFromLeaseholder, false, /* useReverseScan */ | ||
| ) | ||
| require.NoError(t, err) | ||
| require.Len(t, res, 1) | ||
| require.Equal(t, prefixRKey, res[0].StartKey) | ||
| require.Len(t, prefetch, 2) | ||
| require.Equal(t, keys.MustAddr(ten2Split1), prefetch[0].StartKey) | ||
| require.Equal(t, keys.MustAddr(ten2Split2), prefetch[1].StartKey) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // Copyright 2022 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package server | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/keys" | ||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/cockroach/pkg/util/encoding" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestFilterRangeLookupResponseForTenant unit tests the logic to filter | ||
| // RangeLookup connector requests. | ||
| func TestFilterRangeLookupResponseForTenant(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
|
|
||
| ctx := context.Background() | ||
| mkKey := func(tenant uint64, str string) roachpb.RKey { | ||
| codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(tenant)) | ||
| return encoding.EncodeStringAscending(codec.TenantPrefix(), str) | ||
| } | ||
| mkRangeDescriptor := func(start, end roachpb.RKey) roachpb.RangeDescriptor { | ||
| return *roachpb.NewRangeDescriptor(1, start, end, roachpb.MakeReplicaSet(nil)) | ||
| } | ||
| for _, tc := range []struct { | ||
| name string | ||
| id roachpb.TenantID | ||
| descs []roachpb.RangeDescriptor | ||
| exp int | ||
| skipTenantContext bool | ||
| }{ | ||
| // tenant 1, the "system tenant" can see everything. | ||
| { | ||
| name: "tenant 1 is special", | ||
| id: roachpb.MustMakeTenantID(1), | ||
| descs: []roachpb.RangeDescriptor{ | ||
| mkRangeDescriptor(mkKey(1, "a"), mkKey(1, "b")), | ||
| mkRangeDescriptor(mkKey(1, "b"), mkKey(1, "z")), | ||
| mkRangeDescriptor(mkKey(2, "z"), mkKey(2, "")), | ||
| mkRangeDescriptor(mkKey(2, ""), mkKey(2, "a")), | ||
| mkRangeDescriptor(mkKey(2, "a"), mkKey(3, "")), | ||
| mkRangeDescriptor(mkKey(3, ""), mkKey(4, "")), | ||
| }, | ||
| exp: 6, | ||
| }, | ||
| // tenant 2 is a normal secondary tenant and can only see its own data. | ||
| { | ||
| name: "filter to tenant data", | ||
| id: roachpb.MustMakeTenantID(2), | ||
| descs: []roachpb.RangeDescriptor{ | ||
| mkRangeDescriptor(mkKey(2, "a"), mkKey(2, "b")), | ||
| mkRangeDescriptor(mkKey(2, "b"), mkKey(2, "z")), | ||
| mkRangeDescriptor(mkKey(2, "z"), mkKey(3, "")), | ||
| mkRangeDescriptor(mkKey(3, ""), mkKey(3, "a")), | ||
| }, | ||
| exp: 3, | ||
| }, | ||
| // tenant 2 is a normal secondary tenant and can only see its own data, | ||
| // but this includes the case where the range overlaps with multiple | ||
| // tenants. | ||
| { | ||
| name: "filter to tenant data even though range crosses tenants", | ||
| id: roachpb.MustMakeTenantID(2), | ||
| descs: []roachpb.RangeDescriptor{ | ||
| mkRangeDescriptor(mkKey(2, "a"), mkKey(2, "b")), | ||
| mkRangeDescriptor(mkKey(2, "b"), mkKey(2, "z")), | ||
| mkRangeDescriptor(mkKey(2, "z"), mkKey(4, "")), | ||
| mkRangeDescriptor(mkKey(4, ""), mkKey(4, "a")), | ||
| }, | ||
| exp: 3, | ||
| }, | ||
| // If there is no tenant ID in the context, only one result should be | ||
| // returned. | ||
| { | ||
| id: roachpb.MustMakeTenantID(2), | ||
| descs: []roachpb.RangeDescriptor{ | ||
| mkRangeDescriptor(mkKey(2, "a"), mkKey(2, "b")), | ||
| mkRangeDescriptor(mkKey(2, "b"), mkKey(2, "z")), | ||
| mkRangeDescriptor(mkKey(2, "z"), mkKey(3, "")), | ||
| mkRangeDescriptor(mkKey(3, ""), mkKey(3, "a")), | ||
| }, | ||
| skipTenantContext: true, | ||
| exp: 0, | ||
| }, | ||
| // Other code should prevent a request that might return descriptors from | ||
| // another tenant, however, defensively this code should also filter them. | ||
| { | ||
| id: roachpb.MustMakeTenantID(3), | ||
| descs: []roachpb.RangeDescriptor{ | ||
| mkRangeDescriptor(mkKey(2, "a"), mkKey(2, "b")), | ||
| mkRangeDescriptor(mkKey(2, "b"), mkKey(2, "z")), | ||
| mkRangeDescriptor(mkKey(2, "z"), mkKey(3, "")), | ||
| mkRangeDescriptor(mkKey(3, ""), mkKey(3, "a")), | ||
| }, | ||
| exp: 0, | ||
| }, | ||
| } { | ||
| tenantCtx := ctx | ||
| if !tc.skipTenantContext { | ||
| tenantCtx = roachpb.NewContextForTenant(ctx, tc.id) | ||
| } | ||
| got := filterRangeLookupResponseForTenant(tenantCtx, tc.descs) | ||
| require.Len(t, got, tc.exp) | ||
| require.Equal(t, tc.descs[:tc.exp], got) | ||
| } | ||
| } |