Skip to content

Commit

Permalink
workload: remove calls to SanitizeUrls from Ops implementations
Browse files Browse the repository at this point in the history
This commit removes redundant calls to SanitizeUrls from Ops implementations.
These calls are redundant because the urls passed to Ops have already been
sanitized by CmdHelper.

We originally pushed these calls into the Ops implementations in 5bb50b40,
when we had a desire to let individual workloads control their SQL conns.
Passing URLs instead of *sql.DB Ops implementations was a good change. This
additional change to push URL santization into Ops implementations ended up
being unnecessary and led to a lot of duplication.

Epic: None
Release note: None
  • Loading branch information
nvanbenschoten committed Apr 11, 2024
1 parent 87590cc commit 4392d13
Show file tree
Hide file tree
Showing 26 changed files with 6 additions and 95 deletions.
3 changes: 2 additions & 1 deletion pkg/ccl/backupccl/tenant_backup_nemesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ func TestTenantBackupNemesis(t *testing.T) {
return err
}
defer cleanupGoDB()
reg := histogram.NewRegistry(20*time.Second, "bank")
pgURL.Path = bankData.Meta().Name
reg := histogram.NewRegistry(20*time.Second, bankData.Meta().Name)
ops, err := bankData.(workload.Opser).Ops(ctx, []string{pgURL.String()}, reg)
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func TestWorkload(t *testing.T) {

pgURL, cleanup := sqlutils.PGUrl(t, tc.Server(0).AdvSQLAddr(), t.Name(), url.User("testuser"))
defer cleanup()
pgURL.Path = wl.Meta().Name

const concurrency = 2
require.NoError(t, wl.Flags().Parse([]string{
Expand Down
4 changes: 0 additions & 4 deletions pkg/ccl/workloadccl/roachmartccl/roachmart.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,6 @@ func (m *roachmart) Tables() []workload.Table {
func (m *roachmart) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(m, m.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/democluster/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1716,7 +1716,7 @@ func (c *transientCluster) SetupWorkload(ctx context.Context) error {
if err != nil {
return err
}
sqlURLs = append(sqlURLs, sqlURL.ToPQ().String())
sqlURLs = append(sqlURLs, sqlURL.WithDatabase(gen.Meta().Name).ToPQ().String())
}
if err := c.runWorkload(ctx, c.demoCtx.WorkloadGenerator, sqlURLs); err != nil {
return errors.Wrapf(err, "starting background workload")
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/bank/bank.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,6 @@ func (b *bank) Tables() []workload.Table {
func (b *bank) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(b, b.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/bulkingest/bulkingest.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,6 @@ func (w *bulkingest) Tables() []workload.Table {
func (w *bulkingest) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
5 changes: 0 additions & 5 deletions pkg/workload/connectionlatency/connectionlatency.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ func (c *connectionLatency) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
ql := workload.QueryLoad{}
_, err := workload.SanitizeUrls(c, c.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}

for _, url := range urls {
op := &connectionOp{
url: url,
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/indexes/indexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,6 @@ func (w *indexes) Tables() []workload.Table {
func (w *indexes) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
cfg := workload.NewMultiConnPoolCfgFromFlags(w.connFlags)
cfg.MaxTotalConnections = w.connFlags.Concurrency + 1
mcp, err := workload.NewMultiConnPool(ctx, cfg, urls...)
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/insights/insights.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,6 @@ func (b *insights) Tables() []workload.Table {
func (b *insights) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(b, b.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/jsonload/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ func (w *jsonLoad) Tables() []workload.Table {
func (w *jsonLoad) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,10 +449,6 @@ func (w *kv) Tables() []workload.Table {
func (w *kv) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
cfg := workload.NewMultiConnPoolCfgFromFlags(w.connFlags)
cfg.MaxTotalConnections = w.connFlags.Concurrency + 1
mcp, err := workload.NewMultiConnPool(ctx, cfg, urls...)
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/ledger/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,6 @@ func (w *ledger) Tables() []workload.Table {
func (w *ledger) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/movr/workload.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,6 @@ func (m *movr) Ops(
m.fakerOnce.Do(func() {
m.faker = faker.NewFaker()
})
_, err := workload.SanitizeUrls(m, m.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
ql := workload.QueryLoad{}
db, err := workload.NewRoundRobinDB(urls)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/querybench/query_bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ func (*queryBench) Tables() []workload.Table {
func (g *queryBench) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(g, g.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/querylog/querylog.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,6 @@ func (w *querylog) Hooks() workload.Hooks {
func (w *querylog) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ func (w *queue) Tables() []workload.Table {
func (w *queue) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/rand/rand.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,6 @@ func typeForOid(db *gosql.DB, typeOid oid.Oid, tableName, columnName string) (*t
func (w *random) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (ql workload.QueryLoad, retErr error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/schemachange/schemachange.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,6 @@ func (s *schemaChange) Ops(
ctx, span := tracer.Start(ctx, "schemaChange.Ops")
defer func() { EndSpan(span, err) }()

_, err = workload.SanitizeUrls(s, s.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
cfg := workload.NewMultiConnPoolCfgFromFlags(s.connFlags)
// We will need double the concurrency, since we need watch
// dog connections. There is a danger of the pool emptying on
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/sqlsmith/sqlsmith.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ func (g *sqlSmith) Ops(
if err := g.validateErrorSetting(); err != nil {
return workload.QueryLoad{}, err
}
_, err := workload.SanitizeUrls(g, g.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/sqlstats/sqlstats.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ func genPermutations() *gen {
func (s *sqlStats) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(s, s.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/tpcc/tpcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,6 @@ func (w *tpcc) Ops(
w.txCounters = setupTPCCMetrics(reg.Registerer())
}

_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
if err := workload.SetDefaultIsolationLevel(urls, w.isoLevel); err != nil {
return workload.QueryLoad{}, err
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/workload/tpccchecks/checks_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,9 @@ func (*tpccChecks) Meta() workload.Meta {
func (w *tpccChecks) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, errors.Wrapf(err, "could not sanitize urls %v", urls)
}
dbs := make([]*gosql.DB, len(urls))
for i, url := range urls {
var err error
dbs[i], err = gosql.Open(`cockroach`, url)
if err != nil {
return workload.QueryLoad{}, errors.Wrapf(err, "failed to dial %s", url)
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/tpcds/tpcds.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,6 @@ func (w *tpcds) Tables() []workload.Table {
func (w *tpcds) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/tpch/tpch.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,6 @@ func (w *tpch) Tables() []workload.Table {
func (w *tpch) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(w, w.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
4 changes: 0 additions & 4 deletions pkg/workload/ttllogger/ttllogger.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ var logChars = []rune("abdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ !.")
func (l *ttlLogger) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(l, l.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
db, err := gosql.Open(`cockroach`, strings.Join(urls, ` `))
if err != nil {
return workload.QueryLoad{}, err
Expand Down
5 changes: 1 addition & 4 deletions pkg/workload/ycsb/ycsb.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,6 @@ func (g *ycsb) Tables() []workload.Table {
func (g *ycsb) Ops(
ctx context.Context, urls []string, reg *histogram.Registry,
) (workload.QueryLoad, error) {
_, err := workload.SanitizeUrls(g, g.connFlags.DBOverride, urls)
if err != nil {
return workload.QueryLoad{}, err
}
if err := workload.SetDefaultIsolationLevel(urls, g.isoLevel); err != nil {
return workload.QueryLoad{}, err
}
Expand Down Expand Up @@ -442,6 +438,7 @@ func (g *ycsb) Ops(
rowCounter := NewAcknowledgedCounter((uint64)(g.recordCount))

var requestGen randGenerator
var err error
requestGenRng := rand.New(rand.NewSource(RandomSeed.Seed()))
switch strings.ToLower(g.requestDistribution) {
case "zipfian":
Expand Down

0 comments on commit 4392d13

Please sign in to comment.