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

allow TPC-C partition behavior for clients when data is not partitioned #39803

Merged
merged 1 commit into from
Sep 19, 2019
Merged

allow TPC-C partition behavior for clients when data is not partitioned #39803

merged 1 commit into from
Sep 19, 2019

Conversation

sploiselle
Copy link
Contributor

@sploiselle sploiselle commented Aug 21, 2019

Currently, you can only get multiple clients to "collaborate" on a TPC-C run when the actual underlying Cockroach cluster is partitioned. This PR introduces a --client-partition flag that lets a client behave in the same way, even when the data isn't partitioned.

The motivation behind this is showing the degraded TPC-C performance of a CockcroachDB cluster when the data isn't partitioned. With the prior configuration, this wasn't possible.

I believe this belongs in the repo rather than on a branch because similar types of tests might be useful in the future.

Release note: None

Release justification: workload-only change.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@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.

:lgtm: very nice.

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


pkg/workload/tpcc/tpcc.go, line 143 at r1 (raw file):

		))
		g.flags.IntVar(&g.partitions, `partitions`, 1, `Partition tables`)
		g.flags.IntVar(&g.clientPartitions, `client-partitions`, -1, `Make client behave as if the tables are partitioned, but does not actually partition underlying data. Requires --partition-affinity.`)

Default this to 0, since that's what we're using as the condition to indicate that it's not active.


pkg/workload/tpcc/tpcc.go, line 190 at r1 (raw file):

					return errors.Errorf(`cannot specify both --partitions and --client-partitions;
					--partitions actually partitions underlying data.
					--client-partitons only modifies client behavior to access a subset of warehouses. Must be used with --partition-afinity`)

s/partitons/partitions/

s/afinity/affinity/


pkg/workload/tpcc/tpcc.go, line 560 at r1 (raw file):

	}
	var partitionDBs [][]*workload.MultiConnPool

nit: remove this stray line.

@sploiselle
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 16, 2019

Build failed

@nvanbenschoten
Copy link
Member

Could you add:

Release justification: workload-only change.

to your commit message? I think bors might be complaining about that.

Release justification: workload-only change.

Release note: None
@sploiselle
Copy link
Contributor Author

bors r=nvanbenschoten

@craig
Copy link
Contributor

craig bot commented Sep 16, 2019

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Sep 16, 2019

Build failed

@sploiselle sploiselle closed this Sep 17, 2019
@sploiselle sploiselle reopened this Sep 17, 2019
@sploiselle
Copy link
Contributor Author

bors r=nvanbenschoten

@craig
Copy link
Contributor

craig bot commented Sep 19, 2019

Build failed (retrying...)

craig bot pushed a commit that referenced this pull request Sep 19, 2019
39803: allow TPC-C partition behavior for clients when data is not partitioned r=nvanbenschoten a=sploiselle

Currently, you can only get multiple clients to "collaborate" on a TPC-C run when the actual underlying Cockroach cluster is partitioned. This PR introduces a `--client-partition` flag that lets a client behave in the same way, even when the data isn't partitioned.

The motivation behind this is showing the degraded TPC-C performance of a CockcroachDB cluster when the data isn't partitioned. With the prior configuration, this wasn't possible.

I believe this belongs in the repo rather than on a branch because similar types of tests might be useful in the future.

Release note: None

Release justification: workload-only change.

40802: workload: remove index hints from TPC-C r=RaduBerinde a=sploiselle

The heuristic planner benefitted from index hints in some TPC-C statements, but they're now vestigial with the new optimizer. I have removed them to alleviate some of the fussiness of manipulating TPC-C for things like PostgreSQL compatibility.

Release note: None

Release justification: workload-only change.

Co-authored-by: Sean Loiselle <himself@seanloiselle.com>
@craig
Copy link
Contributor

craig bot commented Sep 19, 2019

Build succeeded

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.

3 participants