Skip to content

workload/tpcc: make uuid generation deterministic #37515

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

Merged
merged 1 commit into from
May 15, 2019

Conversation

danhhz
Copy link
Contributor

@danhhz danhhz commented May 14, 2019

Also reduce allocations. Bonus: history table rows are now generated in
primary key order, which the new direct ingestion IMPORT in 19.2 will
take advantage of.

name                               old time/op    new time/op    delta
InitialData/tpcc/warehouses=1-8       385ms ± 1%     355ms ± 0%   -7.80%  (p=0.008 n=5+5)

name                               old speed      new speed      delta
InitialData/tpcc/warehouses=1-8     286MB/s ± 1%   310MB/s ± 0%   +8.45%  (p=0.008 n=5+5)

name                               old alloc/op   new alloc/op   delta
InitialData/tpcc/warehouses=1-8       914kB ± 0%     194kB ± 0%  -78.76%  (p=0.008 n=5+5)

name                               old allocs/op  new allocs/op  delta
InitialData/tpcc/warehouses=1-8       60.6k ± 0%      0.6k ± 0%  -99.02%  (p=0.008 n=5+5)

Release note: None

Also reduce allocations. Bonus: history table rows are now generated in
primary key order, which the new direct ingestion IMPORT in 19.2 will
take advantage of.

    name                               old time/op    new time/op    delta
    InitialData/tpcc/warehouses=1-8       385ms ± 1%     355ms ± 0%   -7.80%  (p=0.008 n=5+5)

    name                               old speed      new speed      delta
    InitialData/tpcc/warehouses=1-8     286MB/s ± 1%   310MB/s ± 0%   +8.45%  (p=0.008 n=5+5)

    name                               old alloc/op   new alloc/op   delta
    InitialData/tpcc/warehouses=1-8       914kB ± 0%     194kB ± 0%  -78.76%  (p=0.008 n=5+5)

    name                               old allocs/op  new allocs/op  delta
    InitialData/tpcc/warehouses=1-8       60.6k ± 0%      0.6k ± 0%  -99.02%  (p=0.008 n=5+5)

Release note: None
@danhhz danhhz requested a review from nvanbenschoten May 14, 2019 22:07
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@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_strong:

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


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

	middleName = []byte(`OE`)
	goodCredit = []byte("GC")
	badCredit  = []byte("BC")

We can remove the cast where these are used now, right?


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

Quoted 4 lines of code…
	// Then, because this involves some floats (and
	// who knows what kind of crazy rounding things can happen when floats are
	// involved), we make sure they're unique by sticking the original row index
	// in the lower 8 bytes.

Good idea!

Copy link
Contributor Author

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

TFTR!

bors r=nvanbenschoten

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


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

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

We can remove the cast where these are used now, right?

already done. one of the two was being conditionally assigned to the credit variable, which lost its cast

craig bot pushed a commit that referenced this pull request May 15, 2019
37515: workload/tpcc: make uuid generation deterministic r=nvanbenschoten a=danhhz

Also reduce allocations. Bonus: history table rows are now generated in
primary key order, which the new direct ingestion IMPORT in 19.2 will
take advantage of.

    name                               old time/op    new time/op    delta
    InitialData/tpcc/warehouses=1-8       385ms ± 1%     355ms ± 0%   -7.80%  (p=0.008 n=5+5)

    name                               old speed      new speed      delta
    InitialData/tpcc/warehouses=1-8     286MB/s ± 1%   310MB/s ± 0%   +8.45%  (p=0.008 n=5+5)

    name                               old alloc/op   new alloc/op   delta
    InitialData/tpcc/warehouses=1-8       914kB ± 0%     194kB ± 0%  -78.76%  (p=0.008 n=5+5)

    name                               old allocs/op  new allocs/op  delta
    InitialData/tpcc/warehouses=1-8       60.6k ± 0%      0.6k ± 0%  -99.02%  (p=0.008 n=5+5)

Release note: None

Co-authored-by: Daniel Harrison <[email protected]>
@craig
Copy link
Contributor

craig bot commented May 15, 2019

Build succeeded

@craig craig bot merged commit 0252c13 into cockroachdb:master May 15, 2019
@danhhz danhhz deleted the workload_tpcc branch May 15, 2019 16:02
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