Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…achdb#116875 cockroachdb#116883

116295: allocator: safe format allocator log messages r=andrewbaptist a=kvoli

See individual commit messages.

Note stringer tests are added in earlier PRs in order to assert against regressions.

Resolves: cockroachdb#102948
Release note:  None

116644: kvclient: remove RPCContext from DistSender r=yuzefovich a=andrewbaptist

RPCContext is unnecessary in DistSender. The only two things that are needed are the LatencyFunc, which is extracted from the RemoteClocks, and the Stopper which should have been passed in directly. This change makes DistSender a tiny bit more easily testable.

Epic: none

Release note: None

116866: go.mod: bump Pebble to bfc74496212f r=dt a=dt

```
cockroachdb/pebble@bfc74496 define FormatSyntheticPrefixes version
cockroachdb/pebble@a27878fb ingest,manifest,sstable: connect prefix rules in ingest to prefix replacing iterators
cockroachdb/pebble@a452c0e5 manifest: add encoding of prefix rules to manifest entries
cockroachdb/pebble@e7d75289 sstable: add prefixReplacingIterator
cockroachdb/pebble@e40df58f sstable: add prefix arg to buildTestTableWithProvider
cockroachdb/pebble@35cfce46 external_iterator: don't double-close Readers in case of err
cockroachdb/pebble@69994ddb sstable: update comment about SINGLEDEL, foreign ssts and compactions
cockroachdb/pebble@40d5f216 db: code comment about guaranteeing durability of initial state
cockroachdb/pebble@5c5ad7ed keyspan: add Assert iterator
cockroachdb/pebble@bbf7dc40 db: deprecate older format versions
```
Release note: none.
Epic: none.

116875: allocatorimpl: check IO overload in should transfer lease r=andrewbaptist a=kvoli

Previously, when the lease IO overload enforcement was set to shed and a store satisfied the IO overload criteria, it would not be guaranteed to have its leases enqueued into the replicate queue. Add an IO overload check in should transfer lease to ensure it does.

Epic: none
Release note: None

116883: application_api: skip more tests under race r=rail a=rickystewart

Epic: CRDB-8308
Release note: None

Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
5 people committed Dec 20, 2023
6 parents 7a7bbff + ea43cc4 + de430df + 5cc5626 + 30da6af + e615aec commit 611607c
Show file tree
Hide file tree
Showing 41 changed files with 572 additions and 216 deletions.
6 changes: 3 additions & 3 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1613,10 +1613,10 @@ def go_deps():
patches = [
"@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
],
sha256 = "1878bb40f322c5c93bb7db26b6287219eb56507fc59b82292fcd4d2187639a16",
strip_prefix = "github.com/cockroachdb/[email protected]20231218155426-48b54c29d8fe",
sha256 = "40cd8262139752f6c002d98be91763eb30cf5d6e2288f7e2163f47ee9b0ca50e",
strip_prefix = "github.com/cockroachdb/[email protected]20231220134654-bfc74496212f",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231218155426-48b54c29d8fe.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231220134654-bfc74496212f.zip",
],
)
go_repository(
Expand Down
2 changes: 1 addition & 1 deletion build/bazelutil/distdir_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/metamorphic/com_github_cockroachdb_metamorphic-v0.0.0-20231108215700-4ba948b56895.zip": "28c8cf42192951b69378cf537be5a9a43f2aeb35542908cc4fe5f689505853ea",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231218155426-48b54c29d8fe.zip": "1878bb40f322c5c93bb7db26b6287219eb56507fc59b82292fcd4d2187639a16",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231220134654-bfc74496212f.zip": "40cd8262139752f6c002d98be91763eb30cf5d6e2288f7e2163f47ee9b0ca50e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220803192808-1806698b1b7b.zip": "3fda531795c600daf25532a4f98be2a1335cd1e5e182c72789bca79f5f69fcc1",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ require (
github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
github.com/cockroachdb/gostdlib v1.19.0
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
github.com/cockroachdb/pebble v0.0.0-20231218155426-48b54c29d8fe
github.com/cockroachdb/pebble v0.0.0-20231220134654-bfc74496212f
github.com/cockroachdb/redact v1.1.5
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895 h1:XANOgPYtvELQ/h4IrmPAohXqe2pWA8Bwhejr3VQoZsA=
github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895/go.mod h1:aPd7gM9ov9M8v32Yy5NJrDyOcD8z642dqs+F0CeNXfA=
github.com/cockroachdb/pebble v0.0.0-20231218155426-48b54c29d8fe h1:ZBhPcgWjnfy2PFWlvPlcOXAfAQqOIdpfksijpKiMWcc=
github.com/cockroachdb/pebble v0.0.0-20231218155426-48b54c29d8fe/go.mod h1:BHuaMa/lK7fUe75BlsteiiTu8ptIG+qSAuDtGMArP18=
github.com/cockroachdb/pebble v0.0.0-20231220134654-bfc74496212f h1:9E2dB7cBwxs8bP4NoZiMEOT6bloFH6kSVhaw0CpkVV0=
github.com/cockroachdb/pebble v0.0.0-20231220134654-bfc74496212f/go.mod h1:BHuaMa/lK7fUe75BlsteiiTu8ptIG+qSAuDtGMArP18=
github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
Expand Down
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ ALL_TESTS = [
"//pkg/kv/kvserver/abortspan:abortspan_test",
"//pkg/kv/kvserver/allocator/allocator2:allocator2_test",
"//pkg/kv/kvserver/allocator/allocatorimpl:allocatorimpl_test",
"//pkg/kv/kvserver/allocator/load:load_test",
"//pkg/kv/kvserver/allocator/storepool:storepool_test",
"//pkg/kv/kvserver/apply:apply_test",
"//pkg/kv/kvserver/asim/gossip:gossip_test",
Expand Down Expand Up @@ -1327,6 +1328,7 @@ GO_TARGETS = [
"//pkg/kv/kvserver/allocator/allocatorimpl:allocatorimpl",
"//pkg/kv/kvserver/allocator/allocatorimpl:allocatorimpl_test",
"//pkg/kv/kvserver/allocator/load:load",
"//pkg/kv/kvserver/allocator/load:load_test",
"//pkg/kv/kvserver/allocator/plan:plan",
"//pkg/kv/kvserver/allocator/storepool:storepool",
"//pkg/kv/kvserver/allocator/storepool:storepool_test",
Expand Down
27 changes: 11 additions & 16 deletions pkg/ccl/cliccl/testdata/ear-list
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,18 @@ list
000004.log:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: d1 05 79 53 68 35 a0 f1 44 01 22 79
counter: 1497766936
nonce: 71 12 f7 22 9a fb 90 24 4e 58 27 01
counter: 3082989236
000005.sst:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: d0 b1 31 4b 08 b9 f6 08 7e e6 af 40
counter: 2167389540
nonce: 5a 01 ec 25 29 aa 75 fc 92 76 48 ad
counter: 1294279409
COCKROACHDB_DATA_KEYS_000001_monolith:
env type: Store, AES128_CTR
keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
nonce: 8f 4c ba 1a a3 4f db 3c db 84 cf f5
counter: 2436226951
CURRENT:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 71 12 f7 22 9a fb 90 24 4e 58 27 01
counter: 3082989236
MANIFEST-000001:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
Expand All @@ -33,20 +28,20 @@ MANIFEST-000001:
OPTIONS-000003:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 86 a7 78 ad 4b da 62 56 d5 e2 d1 70
counter: 798955289
nonce: 80 18 c0 79 61 c7 cf ef b4 25 4e 78
counter: 1483615076
marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith:
env type: Store, AES128_CTR
keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
nonce: 55 d7 d4 27 6c 97 9b dd f1 5d 40 c8
counter: 467030050
marker.format-version.000012.013:
marker.format-version.000001.013:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 23 d9 b2 e1 39 b0 87 ed f9 6d 49 20
counter: 3481614039
nonce: d3 97 11 b3 1a ed 22 2b 74 fb 02 0c
counter: 1229228536
marker.manifest.000001.MANIFEST-000001:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: d3 97 11 b3 1a ed 22 2b 74 fb 02 0c
counter: 1229228536
nonce: 18 c2 a6 23 cc 6e 2e 7c 8e bf 84 77
counter: 3159373900
7 changes: 3 additions & 4 deletions pkg/ccl/storageccl/engineccl/encrypted_fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,10 @@ func TestPebbleEncryption(t *testing.T) {

stats, err := db.GetEnvStats()
require.NoError(t, err)
// Opening the DB should've created OPTIONS, CURRENT, MANIFEST and the
// WAL.
require.GreaterOrEqual(t, stats.TotalFiles, uint64(4))
// Opening the DB should've created OPTIONS, MANIFEST, and the WAL.
require.GreaterOrEqual(t, stats.TotalFiles, uint64(3))
// We also created markers for the format version and the manifest.
require.Equal(t, uint64(6), stats.ActiveKeyFiles)
require.Equal(t, uint64(5), stats.ActiveKeyFiles)
var s enginepbccl.EncryptionStatus
require.NoError(t, protoutil.Unmarshal(stats.EncryptionStatus, &s))
require.Equal(t, "16.key", s.ActiveStoreKey.Source)
Expand Down
39 changes: 19 additions & 20 deletions pkg/kv/kvclient/kvcoord/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ type FirstRangeProvider interface {
type DistSender struct {
log.AmbientContext

st *cluster.Settings
st *cluster.Settings
stopper *stop.Stopper
// clock is used to set time for some calls. E.g. read-only ops
// which span ranges and don't require read consistency.
clock *hlc.Clock
Expand All @@ -529,15 +530,10 @@ type DistSender struct {
// This is not required if a RangeDescriptorDB is supplied.
firstRangeProvider FirstRangeProvider
transportFactory TransportFactory
rpcContext *rpc.Context
// nodeDialer allows RPC calls from the SQL layer to the KV layer.
nodeDialer *nodedialer.Dialer
rpcRetryOptions retry.Options
asyncSenderSem *quotapool.IntPool
// clusterID is the logical cluster ID used to verify access to enterprise features.
// It is copied out of the rpcContext at construction time and used in
// testing.
logicalClusterID *base.ClusterIDContainer

// batchInterceptor is set for tenants; when set, information about all
// BatchRequests and BatchResponses are passed through this interceptor, which
Expand Down Expand Up @@ -588,14 +584,14 @@ type DistSenderConfig struct {
AmbientCtx log.AmbientContext

Settings *cluster.Settings
Stopper *stop.Stopper
Clock *hlc.Clock
NodeDescs NodeDescStore
// NodeIDGetter, if set, provides non-gossip based implementation for
// obtaining the local KV node ID. The DistSender uses the node ID to
// preferentially route requests to a local replica (if one exists).
NodeIDGetter func() roachpb.NodeID
RPCRetryOptions *retry.Options
RPCContext *rpc.Context
// NodeDialer is the dialer from the SQL layer to the KV layer.
NodeDialer *nodedialer.Dialer

Expand Down Expand Up @@ -628,6 +624,8 @@ type DistSenderConfig struct {
TestingKnobs ClientTestingKnobs

HealthFunc HealthFunc

LatencyFunc LatencyFunc
}

// NewDistSender returns a batch.Sender instance which connects to the
Expand All @@ -648,13 +646,15 @@ func NewDistSender(cfg DistSenderConfig) *DistSender {
}
ds := &DistSender{
st: cfg.Settings,
stopper: cfg.Stopper,
clock: cfg.Clock,
nodeDescs: cfg.NodeDescs,
nodeIDGetter: nodeIDGetter,
metrics: makeDistSenderMetrics(),
kvInterceptor: cfg.KVInterceptor,
locality: cfg.Locality,
healthFunc: cfg.HealthFunc,
latencyFunc: cfg.LatencyFunc,
}
if ds.st == nil {
ds.st = cluster.MakeTestingClusterSettings()
Expand All @@ -679,7 +679,7 @@ func NewDistSender(cfg DistSenderConfig) *DistSender {
getRangeDescCacheSize := func() int64 {
return rangeDescriptorCacheSize.Get(&ds.st.SV)
}
ds.rangeCache = rangecache.NewRangeCache(ds.st, rdb, getRangeDescCacheSize, cfg.RPCContext.Stopper)
ds.rangeCache = rangecache.NewRangeCache(ds.st, rdb, getRangeDescCacheSize, cfg.Stopper)
if tf := cfg.TestingKnobs.TransportFactory; tf != nil {
ds.transportFactory = tf
} else {
Expand All @@ -693,21 +693,16 @@ func NewDistSender(cfg DistSenderConfig) *DistSender {
if cfg.RPCRetryOptions != nil {
ds.rpcRetryOptions = *cfg.RPCRetryOptions
}
if cfg.RPCContext == nil {
panic("no RPCContext set in DistSenderConfig")
}
ds.rpcContext = cfg.RPCContext
ds.nodeDialer = cfg.NodeDialer
if ds.rpcRetryOptions.Closer == nil {
ds.rpcRetryOptions.Closer = ds.rpcContext.Stopper.ShouldQuiesce()
ds.rpcRetryOptions.Closer = cfg.Stopper.ShouldQuiesce()
}
ds.logicalClusterID = cfg.RPCContext.LogicalClusterID
ds.asyncSenderSem = quotapool.NewIntPool("DistSender async concurrency",
uint64(senderConcurrencyLimit.Get(&ds.st.SV)))
senderConcurrencyLimit.SetOnChange(&ds.st.SV, func(ctx context.Context) {
ds.asyncSenderSem.UpdateCapacity(uint64(senderConcurrencyLimit.Get(&ds.st.SV)))
})
ds.rpcContext.Stopper.AddCloser(ds.asyncSenderSem.Closer("stopper"))
cfg.Stopper.AddCloser(ds.asyncSenderSem.Closer("stopper"))

if ds.firstRangeProvider != nil {
ctx := ds.AnnotateCtx(context.Background())
Expand All @@ -722,8 +717,12 @@ func NewDistSender(cfg DistSenderConfig) *DistSender {

if cfg.TestingKnobs.LatencyFunc != nil {
ds.latencyFunc = cfg.TestingKnobs.LatencyFunc
} else {
ds.latencyFunc = ds.rpcContext.RemoteClocks.Latency
}
// Some tests don't set the latencyFunc.
if ds.latencyFunc == nil {
ds.latencyFunc = func(roachpb.NodeID) (time.Duration, bool) {
return time.Millisecond, true
}
}

if cfg.TestingKnobs.OnRangeSpanningNonTxnalBatch != nil {
Expand Down Expand Up @@ -1233,9 +1232,9 @@ func (ds *DistSender) divideAndSendParallelCommit(
qiBatchIdx := batchIdx + 1
qiResponseCh := make(chan response, 1)

runTask := ds.rpcContext.Stopper.RunAsyncTask
runTask := ds.stopper.RunAsyncTask
if ds.disableParallelBatches {
runTask = ds.rpcContext.Stopper.RunTask
runTask = ds.stopper.RunTask
}
if err := runTask(ctx, "kv.DistSender: sending pre-commit query intents", func(ctx context.Context) {
// Map response index to the original un-swapped batch index.
Expand Down Expand Up @@ -1776,7 +1775,7 @@ func (ds *DistSender) sendPartialBatchAsync(
responseCh chan response,
positions []int,
) bool {
if err := ds.rpcContext.Stopper.RunAsyncTaskEx(
if err := ds.stopper.RunAsyncTaskEx(
ctx,
stop.TaskOpts{
TaskName: "kv.DistSender: sending partial batch",
Expand Down
6 changes: 1 addition & 5 deletions pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,15 +756,11 @@ func (a *activeRangeFeed) acquireCatchupScanQuota(
func newTransportForRange(
ctx context.Context, desc *roachpb.RangeDescriptor, ds *DistSender,
) (Transport, error) {
var latencyFn LatencyFunc
if ds.rpcContext != nil {
latencyFn = ds.rpcContext.RemoteClocks.Latency
}
replicas, err := NewReplicaSlice(ctx, ds.nodeDescs, desc, nil, AllExtantReplicas)
if err != nil {
return nil, err
}
replicas.OptimizeReplicaOrder(ds.st, ds.nodeIDGetter(), ds.healthFunc, latencyFn, ds.locality)
replicas.OptimizeReplicaOrder(ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality)
opts := SendOptions{class: connectionClass(&ds.st.SV)}
return ds.transportFactory(opts, ds.nodeDialer, replicas)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/dist_sender_rangefeed_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestDistSenderRangeFeedRetryOnTransportErrors(t *testing.T) {
Clock: clock,
NodeDescs: g,
RPCRetryOptions: &retry.Options{MaxRetries: 10},
RPCContext: rpcContext,
Stopper: stopper,
TestingKnobs: ClientTestingKnobs{
TransportFactory: func(SendOptions, *nodedialer.Dialer, ReplicaSlice) (Transport, error) {
return transport, nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvclient/kvcoord/dist_sender_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestRangeLookupWithOpenTransaction(t *testing.T) {
Settings: cluster.MakeTestingClusterSettings(),
Clock: s.Clock(),
NodeDescs: gs,
RPCContext: s.RPCContext(),
Stopper: s.Stopper(),
NodeDialer: nodedialer.New(s.RPCContext(), gossip.AddressResolver(gs)),
FirstRangeProvider: gs,
})
Expand Down Expand Up @@ -1128,7 +1128,7 @@ func TestMultiRangeScanReverseScanInconsistent(t *testing.T) {
Settings: s.ClusterSettings(),
Clock: clock,
NodeDescs: gs,
RPCContext: s.RPCContext(),
Stopper: s.Stopper(),
NodeDialer: nodedialer.New(s.RPCContext(), gossip.AddressResolver(gs)),
FirstRangeProvider: gs,
})
Expand Down Expand Up @@ -1656,7 +1656,7 @@ func TestBatchPutWithConcurrentSplit(t *testing.T) {
AmbientCtx: s.AmbientCtx(),
Clock: s.Clock(),
NodeDescs: gs,
RPCContext: s.RPCContext(),
Stopper: s.Stopper(),
NodeDialer: nodedialer.New(s.RPCContext(), gossip.AddressResolver(gs)),
Settings: cluster.MakeTestingClusterSettings(),
FirstRangeProvider: gs,
Expand Down
Loading

0 comments on commit 611607c

Please sign in to comment.