Skip to content

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Nov 19, 2020

now, we are analyze tpcc workload latency and found current output bucket count is too small and default latency configuration(16s) is too high for our situtation

so this PR try to:

  • replace simple version histogram with https://github.com/HdrHistogram/hdrhistogram-go
  • make maxLatency configurable in tpcc(and keep 16s default value for tpch and other test case)
  • add "efficiency" to tpc-c result to tells us how close database gets to theoretical maximum tpmC

@lysu
Copy link
Contributor Author

lysu commented Nov 19, 2020

@mahjonp @coocood PTAL thx

@coocood
Copy link
Member

coocood commented Nov 19, 2020

LGTM

Copy link
Collaborator

@mahjonp mahjonp left a comment

Choose a reason for hiding this comment

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

Rest LGTM

tpcc/workload.go Outdated
@@ -329,7 +335,10 @@ func (w *Workloader) OutputStats(ifSummaryReport bool) {
hist, e := w.rtMeasurement.OpSumMeasurement["new_order"]
if e && !hist.Empty() {
result := hist.GetInfo()
fmt.Printf("tpmC: %.1f\n", result.Ops*60)
const SpecWarehouseFactor = 12.86
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const SpecWarehouseFactor = 12.86
const specWarehouseFactor = 12.86

@@ -7,10 +7,13 @@ import (
"time"
)

const DefaultMaxLatency = 16 * time.Second
Copy link
Collaborator

Choose a reason for hiding this comment

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

16s is too small for TPC-H and CH-benCHmark, they usually take one minute to ten minutes, and more sf means a longer duration

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems an unreasonable report on TPC-H:

[Summary] Q1: 10.33s
[Summary] Q10: 15.84s
[Summary] Q11: 14.23s
[Summary] Q12: 15.19s
[Summary] Q13: 10.44s
[Summary] Q14: 14.87s
[Summary] Q15: 15.57s
[Summary] Q16: 12.62s
[Summary] Q17: 14.50s
[Summary] Q18: 14.90s
[Summary] Q19: 10.33s
[Summary] Q2: 11.51s
[Summary] Q20: 12.48s
[Summary] Q21: 15.84s
[Summary] Q22: 10.80s
[Summary] Q3: 15.84s
[Summary] Q4: 15.84s
[Summary] Q6: 7.11s
[Summary] Q7: 15.84s
[Summary] Q8: 15.84s
[Summary] Q9: 14.76s

Copy link
Collaborator

@mahjonp mahjonp left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: lysu <sulifx@gmail.com>
@lysu
Copy link
Contributor Author

lysu commented Nov 23, 2020

@mahjonp hi, it has be signed please help merge if free, thx~

@mahjonp mahjonp merged commit 79c4a25 into pingcap:master Nov 23, 2020
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