Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workload: add indexes workload #35077

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

nvanbenschoten
Copy link
Member

Closes #34912.

This adds a new workload called indexes. Unlike kv, the workload is
specifically designed to stress secondary indexes. Its schema looks
like:

CREATE TABLE indexes (
    key     UUID  NOT NULL PRIMARY KEY,
    col0    INT   NOT NULL,
    col1    INT   NOT NULL,
    col2    INT   NOT NULL,
    col3    INT   NOT NULL,
    col4    INT   NOT NULL,
    col5    INT   NOT NULL,
    col6    INT   NOT NULL,
    col7    INT   NOT NULL,
    col8    INT   NOT NULL,
    col9    INT   NOT NULL,
    payload BYTES NOT NULL
)

and it includes knobs to specify how many of the "col" columns should be
indexed and whether they should be indexed using a UNIQUE specifier. The
workload also includes a knob to specify the size of the payload
column.

Release note: None

@nvanbenschoten nvanbenschoten requested review from ajwerner and a team February 19, 2019 22:23
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fine start. I can imagine we'll extend this by adding control for the distribution of writes.

Reviewed 1 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @nvanbenschoten)


pkg/workload/indexes/indexes.go, line 15 at r1 (raw file):

// permissions and limitations under the License.

package bank

package indexes?


pkg/workload/indexes/indexes.go, line 69 at r1 (raw file):

	New: func() workload.Generator {
		g := &indexes{}
		g.flags.FlagSet = pflag.NewFlagSet(`indexes`, pflag.ContinueOnError)

should we add an init flag to be parallel to kv?


pkg/workload/indexes/indexes.go, line 73 at r1 (raw file):

		g.flags.IntVar(&g.idxs, `secondary-indexes`, 1, `Number of indexes to add to the table.`)
		g.flags.BoolVar(&g.unique, `unique-indexes`, false, `Use UNIQUE secondary indexes.`)
		g.flags.IntVar(&g.payload, `payload`, 64, `Size of the payload only present in the primary key.`)

only present in the primary index? I found this documentation sort of confusing.

Copy link
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)


pkg/workload/indexes/indexes.go, line 103 at r1 (raw file):

			}
			for i := 0; i < w.idxs; i++ {
				ddl := fmt.Sprintf(`CREATE%s INDEX idx%d ON indexes (col%d)`, unique, i, i)

unless you're specifically interested in testing schema changes, it's usually better to put these in the table schema. then if you're dealing with fixtures, you get them during the IMPORT

Closes cockroachdb#34912.

This adds a new workload called `indexes`. Unlike `kv`, the workload is
specifically designed to stress secondary indexes. Its schema looks
like:

```
CREATE TABLE indexes (
    key     UUID  NOT NULL PRIMARY KEY,
    col0    INT   NOT NULL,
    col1    INT   NOT NULL,
    col2    INT   NOT NULL,
    col3    INT   NOT NULL,
    col4    INT   NOT NULL,
    col5    INT   NOT NULL,
    col6    INT   NOT NULL,
    col7    INT   NOT NULL,
    col8    INT   NOT NULL,
    col9    INT   NOT NULL,
    payload BYTES NOT NULL
)
```

and it includes knobs to specify how many of the "col" columns should be
indexed and whether they should be indexed using a UNIQUE specifier. The
workload also includes a knob to specify the size of the `payload`
column.

Release note: None
Copy link
Member Author

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)


pkg/workload/indexes/indexes.go, line 15 at r1 (raw file):

Previously, ajwerner wrote…

package indexes?

Done.


pkg/workload/indexes/indexes.go, line 69 at r1 (raw file):

Previously, ajwerner wrote…

should we add an init flag to be parallel to kv?

The init flag is part of workload. See pkg/workload/cli/run.go.


pkg/workload/indexes/indexes.go, line 73 at r1 (raw file):

Previously, ajwerner wrote…

only present in the primary index? I found this documentation sort of confusing.

How's that?


pkg/workload/indexes/indexes.go, line 103 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

unless you're specifically interested in testing schema changes, it's usually better to put these in the table schema. then if you're dealing with fixtures, you get them during the IMPORT

Done.

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@nvanbenschoten
Copy link
Member Author

bors r+

craig bot pushed a commit that referenced this pull request Feb 25, 2019
35077: workload: add indexes workload r=nvanbenschoten a=nvanbenschoten

Closes #34912.

This adds a new workload called `indexes`. Unlike `kv`, the workload is
specifically designed to stress secondary indexes. Its schema looks
like:

```
CREATE TABLE indexes (
    key     UUID  NOT NULL PRIMARY KEY,
    col0    INT   NOT NULL,
    col1    INT   NOT NULL,
    col2    INT   NOT NULL,
    col3    INT   NOT NULL,
    col4    INT   NOT NULL,
    col5    INT   NOT NULL,
    col6    INT   NOT NULL,
    col7    INT   NOT NULL,
    col8    INT   NOT NULL,
    col9    INT   NOT NULL,
    payload BYTES NOT NULL
)
```

and it includes knobs to specify how many of the "col" columns should be
indexed and whether they should be indexed using a UNIQUE specifier. The
workload also includes a knob to specify the size of the `payload`
column.

Release note: None

35116: sql: add support for extra session vars for pg compatibility r=knz a=knz

Fixes #35109.

This adds compatibility support for the following variables, with only
the default values that make sense in CockroachDB:

- `row_security`
- `synchronize_seqscans`,
- `lock_timeout`,
- `idle_in_transaction_session_timeout`

Release note: None

35134: storageccl: leave params in workload URIs r=dt a=dt

workload URIs do not contain anything sensitive and including them in the job make it easier see what it was actually doing.

Release note: none.

35151: sql: tolerate non-existent databases for plan cache invalidation r=knz a=knz

Fixes  #35145.

Release note (bug fix): CockroachDB again properly reports when a
database used during PREPARE does not exist any more when EXECUTE is
used.

35162: storage: remove TODOs in cmd_push_txn r=nvanbenschoten a=nvanbenschoten

This was discussed in #33523 (review) but I missed digging into it at the time. I still don't think I have my head fully wrapped around it, but I do enough to know that it's not something I want to start pulling on right now.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: David Taylor <[email protected]>
@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build succeeded

@craig craig bot merged commit 74e3029 into cockroachdb:master Feb 25, 2019
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/indexes branch February 25, 2019 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants