Skip to content

Commit

Permalink
Merge #100199
Browse files Browse the repository at this point in the history 
100199: Revert "workload: jitter the teardown of connections to prevent thundering herd r=sean- a=sean-

This reverts commit 98e324a.  Need to track down correlated test failures and resolve.

Co-authored-by: Sean Chittenden <sean@chittenden.org>
  • Loading branch information
@sean-
craig[bot] and sean- committed Mar 30, 2023
2 parents 3de803f + 7f433bb commit 8e11ab8
Show file tree
Hide file tree
Showing 31 changed files with 242 additions and 358 deletions.
18 changes: 9 additions & 9 deletions DEPS.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4799,10 +4799,10 @@ def go_deps():
name = "com_github_jackc_pgservicefile",
build_file_proto_mode = "disable_global",
importpath = "github.com/jackc/pgservicefile",
sha256 = "1f8bdf75b2a0d750e56c2a94b1d1b0b5be4b29d6df056aebd997162c29bfd8ab",
strip_prefix = "github.com/jackc/pgservicefile@v0.0.0-20221227161230-091c0ba34f0a",
sha256 = "8422a25b9d2b0be05c66ee1ccfdbaab144ce98f1ac678bc647064c560d4cd6e2",
strip_prefix = "github.com/jackc/pgservicefile@v0.0.0-20200714003250-2b9c44734f2b",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgservicefile/com_github_jackc_pgservicefile-v0.0.0-20221227161230-091c0ba34f0a.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgservicefile/com_github_jackc_pgservicefile-v0.0.0-20200714003250-2b9c44734f2b.zip",
],
)
go_repository(
Expand All @@ -4829,10 +4829,10 @@ def go_deps():
name = "com_github_jackc_pgx_v5",
build_file_proto_mode = "disable_global",
importpath = "github.com/jackc/pgx/v5",
sha256 = "e2f4a98f6b8716a6854d0a910c12c3527d35ff78ec5f2d16bf49f85601071bf0",
strip_prefix = "github.com/jackc/pgx/v5@v5.3.1",
sha256 = "e05b4284fb33e5c0c648b269070dedac6759711c411283177261228ab684f45f",
strip_prefix = "github.com/jackc/pgx/v5@v5.2.0",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v5/com_github_jackc_pgx_v5-v5.3.1.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v5/com_github_jackc_pgx_v5-v5.2.0.zip",
],
)
go_repository(
Expand All @@ -4849,10 +4849,10 @@ def go_deps():
name = "com_github_jackc_puddle_v2",
build_file_proto_mode = "disable_global",
importpath = "github.com/jackc/puddle/v2",
sha256 = "b99ea95df0c0298caf2be786c9eba511bfde2046eccfaa06e89b3e460ab406b0",
strip_prefix = "github.com/jackc/puddle/v2@v2.2.0",
sha256 = "73ea72b52d0a680442d535cf5d9a9713cb0803929c0b4a8e553eda47ee217c44",
strip_prefix = "github.com/jackc/puddle/v2@v2.1.2",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/puddle/v2/com_github_jackc_puddle_v2-v2.2.0.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/puddle/v2/com_github_jackc_puddle_v2-v2.1.2.zip",
],
)
go_repository(
Expand Down
6 changes: 3 additions & 3 deletions build/bazelutil/distdir_files.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,12 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgpassfile/com_github_jackc_pgpassfile-v1.0.0.zip": "1cc79fb0b80f54b568afd3f4648dd1c349f746ad7c379df8d7f9e0eb1cac938b",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/com_github_jackc_pgproto3-v1.1.0.zip": "e3766bee50ed74e49a067b2c4797a2c69015cf104bf3f3624cd483a9e940b4ee",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/v2/com_github_jackc_pgproto3_v2-v2.3.1.zip": "57884e299825af31fd01268659f1e671883b73b708a51230da14d6f8ee0e4e36",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgservicefile/com_github_jackc_pgservicefile-v0.0.0-20221227161230-091c0ba34f0a.zip": "1f8bdf75b2a0d750e56c2a94b1d1b0b5be4b29d6df056aebd997162c29bfd8ab",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgservicefile/com_github_jackc_pgservicefile-v0.0.0-20200714003250-2b9c44734f2b.zip": "8422a25b9d2b0be05c66ee1ccfdbaab144ce98f1ac678bc647064c560d4cd6e2",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgtype/com_github_jackc_pgtype-v1.11.0.zip": "6a257b81c0bd386d6241219a14ebd41d574a02aeaeb3942670c06441b864dcad",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v4/com_github_jackc_pgx_v4-v4.16.1.zip": "c3a169a68ff0e56f9f81eee4de4d2fd2a5ec7f4d6be159159325f4863c80bd10",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v5/com_github_jackc_pgx_v5-v5.3.1.zip": "e2f4a98f6b8716a6854d0a910c12c3527d35ff78ec5f2d16bf49f85601071bf0",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v5/com_github_jackc_pgx_v5-v5.2.0.zip": "e05b4284fb33e5c0c648b269070dedac6759711c411283177261228ab684f45f",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/puddle/com_github_jackc_puddle-v1.2.1.zip": "40d73550686666eb1f6df02b65008b2a4c98cfed1254dc4866e6ebe95fbc5c95",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/puddle/v2/com_github_jackc_puddle_v2-v2.2.0.zip": "b99ea95df0c0298caf2be786c9eba511bfde2046eccfaa06e89b3e460ab406b0",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/puddle/v2/com_github_jackc_puddle_v2-v2.1.2.zip": "73ea72b52d0a680442d535cf5d9a9713cb0803929c0b4a8e553eda47ee217c44",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jaegertracing/jaeger/com_github_jaegertracing_jaeger-v1.18.1.zip": "256a95b2a52a66494aca6d354224bb450ff38ce3ea1890af46a7c8dc39203891",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jcmturner/aescts/v2/com_github_jcmturner_aescts_v2-v2.0.0.zip": "717a211ad4aac248cf33cadde73059c13f8e9462123a0ab2fed5c5e61f7739d7",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jcmturner/dnsutils/v2/com_github_jcmturner_dnsutils_v2-v2.0.0.zip": "f9188186b672e547cfaef66107aa62d65054c5d4f10d4dcd1ff157d6bf8c275d",
Expand Down
4 changes: 1 addition & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ require (
github.com/jackc/pgio v1.0.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgproto3/v2 v2.3.1
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b // indirect
github.com/jackc/pgtype v1.11.0
github.com/jackc/pgx/v4 v4.16.1
github.com/jackc/puddle v1.2.1 // indirect
Expand Down Expand Up @@ -154,7 +154,6 @@ require (
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/guptarohit/asciigraph v0.5.5
github.com/irfansharif/recorder v0.0.0-20211218081646-a21b46510fd6
github.com/jackc/pgx/v5 v5.3.1
github.com/jaegertracing/jaeger v1.18.1
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible
github.com/jordanlewis/gcassert v0.0.0-20221027203946-81f097ad35a0
Expand Down Expand Up @@ -310,7 +309,6 @@ require (
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639 // indirect
github.com/imdario/mergo v0.3.13 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/jackc/puddle/v2 v2.2.0 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
Expand Down
7 changes: 1 addition & 6 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1369,9 +1369,8 @@ github.com/jackc/pgproto3/v2 v2.1.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwX
github.com/jackc/pgproto3/v2 v2.3.0/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgproto3/v2 v2.3.1 h1:nwj7qwf0S+Q7ISFfBndqeLwSwxs+4DPsbRFjECT1Y4Y=
github.com/jackc/pgproto3/v2 v2.3.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b h1:C8S2+VttkHFdOOCXJe+YGfa4vHYwlt4Zx+IVXQ97jYg=
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E=
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a h1:bbPeKD0xmW/Y25WS6cokEszi5g+S0QxI/d45PkRi7Nk=
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
github.com/jackc/pgtype v0.0.0-20190421001408-4ed0de4755e0/go.mod h1:hdSHsc1V01CGwFsrv11mJRHWJ6aifDLfdV3aVjFF0zg=
github.com/jackc/pgtype v0.0.0-20190824184912-ab885b375b90/go.mod h1:KcahbBH1nCMSo2DXpzsoWOAfFkdEtEJpPbVLq8eE+mc=
github.com/jackc/pgtype v0.0.0-20190828014616-a8802b16cc59/go.mod h1:MWlu30kVJrUS8lot6TQqcg7mtthZ9T0EoIBFiJcmcyw=
Expand All @@ -1384,15 +1383,11 @@ github.com/jackc/pgx/v4 v4.0.0-pre1.0.20190824185557-6972a5742186/go.mod h1:X+GQ
github.com/jackc/pgx/v4 v4.12.1-0.20210724153913-640aa07df17c/go.mod h1:1QD0+tgSXP7iUjYm9C1NxKhny7lq6ee99u/z+IHFcgs=
github.com/jackc/pgx/v4 v4.16.1 h1:JzTglcal01DrghUqt+PmzWsZx/Yh7SC/CTQmSBMTd0Y=
github.com/jackc/pgx/v4 v4.16.1/go.mod h1:SIhx0D5hoADaiXZVyv+3gSm3LCIIINTVO0PficsvWGQ=
github.com/jackc/pgx/v5 v5.3.1 h1:Fcr8QJ1ZeLi5zsPZqQeUZhNhxfkkKBOgJuYkJHoBOtU=
github.com/jackc/pgx/v5 v5.3.1/go.mod h1:t3JDKnCBlYIc0ewLF0Q7B8MXmoIaBOZj/ic7iHozM/8=
github.com/jackc/puddle v0.0.0-20190413234325-e4ced69a3a2b/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v0.0.0-20190608224051-11cab39313c9/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v1.1.3/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v1.2.1 h1:gI8os0wpRXFd4FiAY2dWiqRK037tjj3t7rKFeO4X5iw=
github.com/jackc/puddle v1.2.1/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle/v2 v2.2.0 h1:RdcDk92EJBuBS55nQMMYFXTxwstHug4jkhT5pq8VxPk=
github.com/jackc/puddle/v2 v2.2.0/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
github.com/jaegertracing/jaeger v1.18.1 h1:eFqjEpTKq2FfiZ/YX53oxeCePdIZyWvDfXaTAGj0r5E=
github.com/jaegertracing/jaeger v1.18.1/go.mod h1:WRzMFH62rje1VgbShlgk6UbWUNoo08uFFvs/x50aZKk=
github.com/jcmturner/aescts/v2 v2.0.0 h1:9YKLH6ey7H4eDBXW8khjYslgyqG2xZikXP0EQFKrle8=
Expand Down
7 changes: 3 additions & 4 deletions pkg/workload/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ go_library(
"//pkg/util/timeutil",
"//pkg/workload/histogram",
"@com_github_cockroachdb_errors//:errors",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_jackc_pgx_v5//pgxpool",
"@com_github_jackc_pgx_v5//tracelog",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v4//pgxpool",
"@com_github_lib_pq//:pq",
"@com_github_spf13_pflag//:pflag",
"@org_golang_x_sync//errgroup",
Expand Down
19 changes: 3 additions & 16 deletions pkg/workload/connection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"net/url"
"runtime"
"strings"
"time"

"github.com/spf13/pflag"
)
Expand All @@ -25,14 +24,8 @@ type ConnFlags struct {
*pflag.FlagSet
DBOverride string
Concurrency int
Method string // Method for issuing queries; see SQLRunner.

ConnHealthCheckPeriod time.Duration
MaxConnIdleTime time.Duration
MaxConnLifetime time.Duration
MaxConnLifetimeJitter time.Duration
MinConns int
WarmupConns int
// Method for issuing queries; see SQLRunner.
Method string
}

// NewConnFlags returns an initialized ConnFlags.
Expand All @@ -43,13 +36,7 @@ func NewConnFlags(genFlags *Flags) *ConnFlags {
`Override for the SQL database to use. If empty, defaults to the generator name`)
c.IntVar(&c.Concurrency, `concurrency`, 2*runtime.GOMAXPROCS(0),
`Number of concurrent workers`)
c.StringVar(&c.Method, `method`, `cache_statement`, `SQL issue method (cache_statement, cache_describe, describe_exec, exec, simple_protocol)`)
c.DurationVar(&c.ConnHealthCheckPeriod, `conn-healthcheck-period`, 30*time.Second, `Interval that health checks are run on connections`)
c.IntVar(&c.MinConns, `min-conns`, 0, `Minimum number of connections to attempt to keep in the pool`)
c.DurationVar(&c.MaxConnIdleTime, `max-conn-idle-time`, 150*time.Second, `Max time an idle connection will be kept around`)
c.DurationVar(&c.MaxConnLifetime, `max-conn-lifetime`, 300*time.Second, `Max connection lifetime`)
c.DurationVar(&c.MaxConnLifetimeJitter, `max-conn-lifetime-jitter`, 150*time.Second, `Jitter max connection lifetime by this amount`)
c.IntVar(&c.WarmupConns, `warmup-conns`, 0, `Number of connections to warmup in each connection pool`)
c.StringVar(&c.Method, `method`, `prepare`, `SQL issue method (prepare, noprepare, simple)`)
genFlags.AddFlagSet(c.FlagSet)
if genFlags.Meta == nil {
genFlags.Meta = make(map[string]FlagMeta)
Expand Down
2 changes: 1 addition & 1 deletion pkg/workload/connectionlatency/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ go_library(
"//pkg/util/timeutil",
"//pkg/workload",
"//pkg/workload/histogram",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_spf13_pflag//:pflag",
],
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/workload/connectionlatency/connectionlatency.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/workload"
"github.com/cockroachdb/cockroach/pkg/workload/histogram"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v4"
"github.com/spf13/pflag"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/workload/histogram/histogram.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ func (w *Registry) newNamedHistogramLocked(name string) *NamedHistogram {
// Record saves a new datapoint and should be called once per logical operation.
func (w *NamedHistogram) Record(elapsed time.Duration) {
w.prometheusHistogram.Observe(float64(elapsed.Nanoseconds()) / float64(time.Second))
w.mu.Lock()
maxLatency := time.Duration(w.mu.current.HighestTrackableValue())
if elapsed < minLatency {
elapsed = minLatency
} else if elapsed > maxLatency {
elapsed = maxLatency
}

w.mu.Lock()
err := w.mu.current.RecordValue(elapsed.Nanoseconds())
w.mu.Unlock()

Expand Down
7 changes: 4 additions & 3 deletions pkg/workload/indexes/indexes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,9 @@ func (w *indexes) Ops(
if err != nil {
return workload.QueryLoad{}, err
}
cfg := workload.NewMultiConnPoolCfgFromFlags(w.connFlags)
cfg.MaxTotalConnections = w.connFlags.Concurrency + 1
cfg := workload.MultiConnPoolCfg{
MaxTotalConnections: w.connFlags.Concurrency + 1,
}
mcp, err := workload.NewMultiConnPool(ctx, cfg, urls...)
if err != nil {
return workload.QueryLoad{}, err
Expand All @@ -175,7 +176,7 @@ func (w *indexes) Ops(
buf: make([]byte, w.payload),
}
op.stmt = op.sr.Define(stmt)
if err := op.sr.Init(ctx, "indexes", mcp); err != nil {
if err := op.sr.Init(ctx, "indexes", mcp, w.connFlags); err != nil {
return workload.QueryLoad{}, err
}
ql.WorkerFns = append(ql.WorkerFns, op.run)
Expand Down
4 changes: 2 additions & 2 deletions pkg/workload/kv/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ go_library(
"//pkg/workload",
"//pkg/workload/histogram",
"@com_github_cockroachdb_errors//:errors",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_spf13_pflag//:pflag",
],
)
Expand Down
15 changes: 7 additions & 8 deletions pkg/workload/kv/kv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/workload"
"github.com/cockroachdb/cockroach/pkg/workload/histogram"
"github.com/cockroachdb/errors"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/jackc/pgconn"
"github.com/jackc/pgx/v4"
"github.com/spf13/pflag"
)

Expand Down Expand Up @@ -347,8 +347,9 @@ func (w *kv) Ops(
if err != nil {
return workload.QueryLoad{}, err
}
cfg := workload.NewMultiConnPoolCfgFromFlags(w.connFlags)
cfg.MaxTotalConnections = w.connFlags.Concurrency + 1
cfg := workload.MultiConnPoolCfg{
MaxTotalConnections: w.connFlags.Concurrency + 1,
}
mcp, err := workload.NewMultiConnPool(ctx, cfg, urls...)
if err != nil {
return workload.QueryLoad{}, err
Expand Down Expand Up @@ -444,7 +445,7 @@ func (w *kv) Ops(
}
op.spanStmt = op.sr.Define(spanStmtStr)
op.delStmt = op.sr.Define(delStmtStr)
if err := op.sr.Init(ctx, "kv", mcp); err != nil {
if err := op.sr.Init(ctx, "kv", mcp, w.connFlags); err != nil {
return workload.QueryLoad{}, err
}
op.mcp = mcp
Expand Down Expand Up @@ -556,11 +557,9 @@ func (o *kvOp) run(ctx context.Context) (retErr error) {
// that each run call makes 1 attempt, so that rate limiting in workerRun
// behaves as expected.
var tx pgx.Tx
tx, err := o.mcp.Get().BeginTx(ctx, pgx.TxOptions{})
if err != nil {
if tx, err = o.mcp.Get().Begin(ctx); err != nil {
return err
}

defer func() {
rollbackErr := tx.Rollback(ctx)
if !errors.Is(rollbackErr, pgx.ErrTxClosed) {
Expand Down
Loading

0 comments on commit 8e11ab8

Please sign in to comment.