From 381df3a2aebb7616e73cb2e3902261f181f135bb Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Fri, 22 Jul 2022 16:00:13 -0400 Subject: [PATCH 1/3] vendor: bump Pebble to 89b7bc729bd1 89b7bc72 db: lazily construct combined iterator 4adbba57 internal/base: add SeekLTFlags 97fbe8e2 internal/base: replace trySeekUsingNext with SeekGEFlags 63d55279 lint: use `go run path@version` syntax for lint binaries Release note: None --- DEPS.bzl | 12 ++++++------ build/bazelutil/distdir_files.bzl | 4 ++-- go.mod | 2 +- go.sum | 5 +++-- pkg/storage/mvcc_history_test.go | 2 +- pkg/storage/sst_iterator.go | 8 ++++---- vendor | 2 +- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index a1d34133745a..f1b95f6ac295 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -1431,10 +1431,10 @@ def go_deps(): patches = [ "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch", ], - sha256 = "ba44b4818846bee833b98b11e22efa4c04efed9533ba16e8e7013a52a5cbc1ab", - strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20220718171027-6b9be031039a", + sha256 = "a608aea7391e1369292d67d97ed4f9cbc19deb5f402c815982de420e1ef6170f", + strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20220722194627-89b7bc729bd1", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20220718171027-6b9be031039a.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20220722194627-89b7bc729bd1.zip", ], ) go_repository( @@ -4025,10 +4025,10 @@ def go_deps(): name = "com_github_hashicorp_go_version", build_file_proto_mode = "disable_global", importpath = "github.com/hashicorp/go-version", - sha256 = "a3231adb6bf029750970de2955e82e41e4c062b94eb73683e9111aa0c0841008", - strip_prefix = "github.com/hashicorp/go-version@v1.2.0", + sha256 = "bf1d96bda50abf5e2d111bf99d220d978314907d815fd58f4bd4770dc7959b9e", + strip_prefix = "github.com/hashicorp/go-version@v1.6.0", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-version/com_github_hashicorp_go_version-v1.2.0.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-version/com_github_hashicorp_go_version-v1.6.0.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index ac523b62447c..2cf8bc709b3f 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -187,7 +187,7 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.13.0.zip": "b3d43d8f95edf65f73a5348f29e1159823cac64b148f8d3bb48340bf55d70872", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20211118104740-dabe8e521a4f.zip": "1972c3f171f118add3fd9e64bcea6cbb9959a3b7fa0ada308e8a7310813fea74", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20220718171027-6b9be031039a.zip": "ba44b4818846bee833b98b11e22efa4c04efed9533ba16e8e7013a52a5cbc1ab", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20220722194627-89b7bc729bd1.zip": "a608aea7391e1369292d67d97ed4f9cbc19deb5f402c815982de420e1ef6170f", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.3.zip": "7778b1e4485e4f17f35e5e592d87eb99c29e173ac9507801d000ad76dd0c261e", "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/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9", @@ -441,7 +441,7 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-sockaddr/com_github_hashicorp_go_sockaddr-v1.0.2.zip": "50c1b60863b0cd31d03b26d3975f76cab55466666c067cd1823481a61f19af33", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-syslog/com_github_hashicorp_go_syslog-v1.0.0.zip": "a0ca8b61ea365e9ecdca513b94f200aef3ff68b4c95d9dabc88ca25fcb33bce6", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-uuid/com_github_hashicorp_go_uuid-v1.0.3.zip": "5e9dc2bb3785d69a65d287a4b3fa7e9f583a127e41c6a2fd095ac862fed71dad", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-version/com_github_hashicorp_go_version-v1.2.0.zip": "a3231adb6bf029750970de2955e82e41e4c062b94eb73683e9111aa0c0841008", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go-version/com_github_hashicorp_go_version-v1.6.0.zip": "bf1d96bda50abf5e2d111bf99d220d978314907d815fd58f4bd4770dc7959b9e", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/go.net/com_github_hashicorp_go_net-v0.0.1.zip": "71564aa3cb6e2820ee31e4d9e264e4ed889c7916f958b2f54c6f3004d4fcd8d2", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/golang-lru/com_github_hashicorp_golang_lru-v0.5.4.zip": "7b2a8b1739c858727fca497a6415323edb801dc97b8aca04f7bac4ab9fb5c66b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/hashicorp/hcl/com_github_hashicorp_hcl-v1.0.0.zip": "54149a2e5121b3e81f961c79210e63d6798eb63de28d2599ee59ade1fa76c82b", diff --git a/go.mod b/go.mod index 22fcf49aa4e2..e451bd852c1a 100644 --- a/go.mod +++ b/go.mod @@ -47,7 +47,7 @@ require ( github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55 github.com/cockroachdb/gostdlib v1.13.0 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f - github.com/cockroachdb/pebble v0.0.0-20220718171027-6b9be031039a + github.com/cockroachdb/pebble v0.0.0-20220722194627-89b7bc729bd1 github.com/cockroachdb/redact v1.1.3 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd github.com/cockroachdb/stress v0.0.0-20220310203902-58fb4627376e diff --git a/go.sum b/go.sum index 082f625f33ee..5e7ad62d80e5 100644 --- a/go.sum +++ b/go.sum @@ -468,8 +468,8 @@ github.com/cockroachdb/gostdlib v1.13.0/go.mod h1:eXX95p9QDrYwJfJ6AgeN9QnRa/lqqi github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI= github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f h1:6jduT9Hfc0njg5jJ1DdKCFPdMBrp/mdZfCpa5h+WM74= github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= -github.com/cockroachdb/pebble v0.0.0-20220718171027-6b9be031039a h1:zZ4qaemk1HqFFlQsYBFW88JljJirycMZq2tiG3UP3AE= -github.com/cockroachdb/pebble v0.0.0-20220718171027-6b9be031039a/go.mod h1:pr479tNxFRmcfDyklTqoRMDDVmRlEbL+d7a7rhKnrI4= +github.com/cockroachdb/pebble v0.0.0-20220722194627-89b7bc729bd1 h1:9pOMH6J3ybV793me7yD6A24KEBlyJKY+Yje2ZMrZEnU= +github.com/cockroachdb/pebble v0.0.0-20220722194627-89b7bc729bd1/go.mod h1:890yq1fUb9b6dGNwssgeUO5vQV9qfXnCPxAJhBQfXw0= github.com/cockroachdb/redact v1.0.8/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= github.com/cockroachdb/redact v1.1.3 h1:AKZds10rFSIj7qADf0g46UixK8NNLwWTNdCIGS5wfSQ= github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= @@ -1229,6 +1229,7 @@ github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/b github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= +github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go.net v0.0.1/go.mod h1:hjKkEWcCURg++eb33jQU7oqQcI9XDCnUzHA0oac0k90= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= diff --git a/pkg/storage/mvcc_history_test.go b/pkg/storage/mvcc_history_test.go index c192765849ef..275f0d2f3a75 100644 --- a/pkg/storage/mvcc_history_test.go +++ b/pkg/storage/mvcc_history_test.go @@ -232,7 +232,7 @@ func TestMVCCHistories(t *testing.T) { return err } defer func() { _ = iter.Close() }() - for k, v := iter.SeekGE(nil, false); k != nil; k, v = iter.Next() { + for k, v := iter.SeekGE(nil, sstable.SeekGEFlags(0)); k != nil; k, v = iter.Next() { if err := iter.Error(); err != nil { return err } diff --git a/pkg/storage/sst_iterator.go b/pkg/storage/sst_iterator.go index a147ae1457f9..2c2079f5597e 100644 --- a/pkg/storage/sst_iterator.go +++ b/pkg/storage/sst_iterator.go @@ -132,15 +132,15 @@ func (r *sstIterator) SeekGE(key MVCCKey) { } r.keyBuf = EncodeMVCCKeyToBuf(r.keyBuf, key) var iKey *sstable.InternalKey - trySeekUsingNext := false - if r.seekGELastOp { + var flags sstable.SeekGEFlags + if r.seekGELastOp && !key.Less(r.prevSeekKey) { // trySeekUsingNext = r.prevSeekKey <= key - trySeekUsingNext = !key.Less(r.prevSeekKey) + flags = flags.EnableTrySeekUsingNext() } // NB: seekGELastOp may still be true, and we haven't updated prevSeekKey. // So be careful not to return before the end of the function that sets these // fields up for the next SeekGE. - iKey, r.value = r.iter.SeekGE(r.keyBuf, trySeekUsingNext) + iKey, r.value = r.iter.SeekGE(r.keyBuf, flags) if iKey != nil { r.iterValid = true r.mvccKey, r.err = DecodeMVCCKey(iKey.UserKey) diff --git a/vendor b/vendor index 422a67489ca4..cc3365338f7c 160000 --- a/vendor +++ b/vendor @@ -1 +1 @@ -Subproject commit 422a67489ca47b7ca2c6aedfbc3cb8a7586d0df0 +Subproject commit cc3365338f7c8efce00219b02ed257a9d26d5dcb From 9692b54d467e464c3dd1a7a2c96d9d2a2a1c489a Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Fri, 22 Jul 2022 14:47:00 -0700 Subject: [PATCH 2/3] distsql: reserve 10KiB memory on admission of remote flow This commit makes it so that we reserve 10KiB of memory when scheduling each remote DistSQL flow. The idea is that a read-only remote flow (since we don't distribute writes) will need to account for some memory anyway, so we might as well make a small reservation upfront. This would serve as a form of admission control "based" on the RAM usage. Release note: None --- pkg/sql/distsql/server.go | 63 +++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/pkg/sql/distsql/server.go b/pkg/sql/distsql/server.go index 230c93970554..824861ad4437 100644 --- a/pkg/sql/distsql/server.go +++ b/pkg/sql/distsql/server.go @@ -203,6 +203,9 @@ func FlowVerIsCompatible( // setupFlow creates a Flow. // // Args: +// reserved: Specifies the upfront memory reservation that the flow takes +// ownership of. This account is already closed if an error is returned or +// will be closed through Flow.Cleanup. // localState: Specifies if the flow runs entirely on this node and, if it does, // specifies the txn and other attributes. // @@ -212,23 +215,17 @@ func (ds *ServerImpl) setupFlow( ctx context.Context, parentSpan *tracing.Span, parentMonitor *mon.BytesMonitor, + reserved *mon.BoundAccount, req *execinfrapb.SetupFlowRequest, rowSyncFlowConsumer execinfra.RowReceiver, batchSyncFlowConsumer execinfra.BatchReceiver, localState LocalState, ) (retCtx context.Context, _ flowinfra.Flow, _ execopnode.OpChains, retErr error) { - if !FlowVerIsCompatible(req.Version, execinfra.MinAcceptedVersion, execinfra.Version) { - err := errors.Errorf( - "version mismatch in flow request: %d; this node accepts %d through %d", - req.Version, execinfra.MinAcceptedVersion, execinfra.Version, - ) - log.Warningf(ctx, "%v", err) - return ctx, nil, nil, err - } - var sp *tracing.Span // will be Finish()ed by Flow.Cleanup() var monitor *mon.BytesMonitor // will be closed in Flow.Cleanup() - var onFlowCleanup func() + onFlowCleanup := func() { + reserved.Close(retCtx) + } // Make sure that we clean up all resources (which in the happy case are // cleaned up in Flow.Cleanup()) if an error is encountered. defer func() { @@ -239,13 +236,20 @@ func (ds *ServerImpl) setupFlow( if monitor != nil { monitor.Stop(ctx) } - if onFlowCleanup != nil { - onFlowCleanup() - } + onFlowCleanup() retCtx = tracing.ContextWithSpan(ctx, nil) } }() + if !FlowVerIsCompatible(req.Version, execinfra.MinAcceptedVersion, execinfra.Version) { + err := errors.Errorf( + "version mismatch in flow request: %d; this node accepts %d through %d", + req.Version, execinfra.MinAcceptedVersion, execinfra.Version, + ) + log.Warningf(ctx, "%v", err) + return ctx, nil, nil, err + } + const opName = "flow" if parentSpan == nil { ctx, sp = ds.Tracer.StartSpanCtx(ctx, opName) @@ -275,7 +279,7 @@ func (ds *ServerImpl) setupFlow( noteworthyMemoryUsageBytes, ds.Settings, ) - monitor.StartNoReserved(ctx, parentMonitor) + monitor.Start(ctx, parentMonitor, reserved) makeLeaf := func() (*kv.Txn, error) { tis := req.LeafTxnInputState @@ -307,9 +311,11 @@ func (ds *ServerImpl) setupFlow( // the original state in order to avoid performance regressions. origMon := evalCtx.Mon origTxn := evalCtx.Txn + oldOnFlowCleanup := onFlowCleanup onFlowCleanup = func() { evalCtx.Mon = origMon evalCtx.Txn = origTxn + oldOnFlowCleanup() } evalCtx.Mon = monitor if localState.MustUseLeafTxn() { @@ -559,7 +565,8 @@ func (ds *ServerImpl) SetupLocalSyncFlow( localState LocalState, ) (context.Context, flowinfra.Flow, execopnode.OpChains, error) { ctx, f, opChains, err := ds.setupFlow( - ctx, tracing.SpanFromContext(ctx), parentMonitor, req, output, batchOutput, localState, + ctx, tracing.SpanFromContext(ctx), parentMonitor, &mon.BoundAccount{}, /* reserved */ + req, output, batchOutput, localState, ) if err != nil { return nil, nil, nil, err @@ -615,14 +622,24 @@ func (ds *ServerImpl) SetupFlow( // Note: the passed context will be canceled when this RPC completes, so we // can't associate it with the flow. ctx = ds.AnnotateCtx(context.Background()) - ctx, f, _, err := ds.setupFlow( - ctx, rpcSpan, ds.memMonitor, req, nil, /* rowSyncFlowConsumer */ - nil /* batchSyncFlowConsumer */, LocalState{}, - ) - if err == nil { - err = ds.flowScheduler.ScheduleFlow(ctx, f) - } - if err != nil { + if err := func() error { + // Reserve some memory for this remote flow which is a poor man's + // admission control based on the RAM usage. + reserved := ds.memMonitor.MakeBoundAccount() + err := reserved.Grow(ctx, mon.DefaultPoolAllocationSize) + if err != nil { + return err + } + var f flowinfra.Flow + ctx, f, _, err = ds.setupFlow( + ctx, rpcSpan, ds.memMonitor, &reserved, req, nil, /* rowSyncFlowConsumer */ + nil /* batchSyncFlowConsumer */, LocalState{}, + ) + if err != nil { + return err + } + return ds.flowScheduler.ScheduleFlow(ctx, f) + }(); err != nil { // We return flow deployment errors in the response so that they are // packaged correctly over the wire. If we return them directly to this // function, they become part of an rpc error. From ed0c24d0da6c193c6a680c8300e133d5d6c86a06 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Fri, 22 Jul 2022 22:56:15 +0000 Subject: [PATCH 3/3] dev: v45 ./dev's version wasn't properly incremented in PR 84766 [1] due to a last-minute rebase, in which someone else increased the version as well. Bump dev's version to force a rebuild. [1] https://github.com/cockroachdb/cockroach/pull/84766 Release note: None --- dev | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev b/dev index 3e9e7c4147b4..644c5490cc46 100755 --- a/dev +++ b/dev @@ -3,7 +3,7 @@ set -euo pipefail # Bump this counter to force rebuilding `dev` on all machines. -DEV_VERSION=44 +DEV_VERSION=45 THIS_DIR=$(cd "$(dirname "$0")" && pwd) BINARY_DIR=$THIS_DIR/bin/dev-versions