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

executor, util: improve concurrency of statement summary #14490

Merged
merged 9 commits into from
Feb 4, 2020

Conversation

djshow832
Copy link
Contributor

@djshow832 djshow832 commented Jan 16, 2020

What problem does this PR solve?

Improve the performance in point get.
In point_get of wide_table_200, QPS decreases from 59k to 49k (refer to issue #14432). Because there're too many kinds of summaries to fit in the cache, creating a new summary needs to lock the cache, so much time is spent on waiting for the lock.
Also, some time is spent on generating digest.

After this PR, QPS increases back to 57k.

What is changed and how it works?

  • Move some calculation out of cache lock to decrease lock contention.
  • If it's a point_get, only generate digest once.

Check List

Tests

  • Unit test

Code changes

N/A

Side effects

  • Increased code complexity

Related changes

N/A

Release note

  • Improve concurrency of statement summary by decreasing lock contention.

@djshow832 djshow832 force-pushed the stmt_summary_improve_concurrency branch from 0f332bd to 9ceaa9b Compare January 16, 2020 04:40
@djshow832
Copy link
Contributor Author

/bench + sysbench + tpcc

@sre-bot
Copy link
Contributor

sre-bot commented Jan 16, 2020

Benchmark Report

Run TPC-C Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: e39b504177da2efee6a8981b31c45ac775c03612
+++ tidb: 9ceaa9b3ed8de92e66962196f99fb9ed97f5b0fa
tikv: 18a4fe02b916eb4f96873e5e92ad02ec349077a4
pd: 541e3a57cd996bc46e91def9eb5437b05b5da6b0
================================================================================
Measured tpmC (NewOrders): 6597.87 ± 0.09% (std=4.30), delta: -0.60% (p=0.866)

@sre-bot
Copy link
Contributor

sre-bot commented Jan 16, 2020

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: e39b504177da2efee6a8981b31c45ac775c03612
+++ tidb: 9ceaa9b3ed8de92e66962196f99fb9ed97f5b0fa
tikv: 18a4fe02b916eb4f96873e5e92ad02ec349077a4
pd: 541e3a57cd996bc46e91def9eb5437b05b5da6b0
================================================================================
oltp_insert:
    * QPS: 7430.87 ± 0.29% (std=15.38) delta: 0.16% (p=0.243)
    * Latency p50: 17.22 ± 0.29% (std=0.04) delta: -0.16%
    * Latency p99: 29.32 ± 2.23% (std=0.44) delta: -0.44%
            
oltp_point_select:
    * QPS: 43952.32 ± 0.06% (std=18.98) delta: 1.18% (p=0.429)
    * Latency p50: 2.91 ± 0.00% (std=0.00) delta: -0.23%
    * Latency p99: 9.91 ± 0.00% (std=0.00) delta: 0.00%
            
oltp_read_write:
    * QPS: 15891.28 ± 0.20% (std=27.82) delta: -0.07% (p=0.681)
    * Latency p50: 161.43 ± 0.20% (std=0.25) delta: 0.05%
    * Latency p99: 306.13 ± 2.72% (std=6.16) delta: 1.53%
            
oltp_update_index:
    * QPS: 4203.73 ± 0.57% (std=14.79) delta: -0.07% (p=0.412)
    * Latency p50: 30.45 ± 0.57% (std=0.11) delta: 0.10%
    * Latency p99: 52.91 ± 3.63% (std=1.51) delta: -1.16%
            
oltp_update_non_index:
    * QPS: 4722.89 ± 0.30% (std=10.55) delta: 0.02% (p=0.925)
    * Latency p50: 27.10 ± 0.30% (std=0.06) delta: -0.02%
    * Latency p99: 41.85 ± 0.00% (std=0.00) delta: -1.36%
            

@djshow832
Copy link
Contributor Author

/run-unit-test

@zyxbest
Copy link
Contributor

zyxbest commented Jan 16, 2020

/rebuild

@zyxbest
Copy link
Contributor

zyxbest commented Jan 16, 2020

/build

@djshow832
Copy link
Contributor Author

/bench + sysbench + tpcc

@djshow832 djshow832 changed the title util: improve concurrency of statement summary executor, util: improve concurrency of statement summary Jan 17, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 17, 2020

Benchmark Report

Run TPC-C Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 74bc00dfa953da06d8e5823d0acbf6bf1858a680
+++ tidb: 0b5f79dc3a4ee74ce6abc90f9242c00d4324db61
tikv: c75e85a7535127b4373514f0c80fc631dd30b7b7
pd: 541e3a57cd996bc46e91def9eb5437b05b5da6b0
================================================================================
Measured tpmC (NewOrders): 6624.33 ± 1.08% (std=52.15), delta: 1.17% (p=0.086)

@sre-bot
Copy link
Contributor

sre-bot commented Jan 17, 2020

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 74bc00dfa953da06d8e5823d0acbf6bf1858a680
+++ tidb: 0b5f79dc3a4ee74ce6abc90f9242c00d4324db61
tikv: c75e85a7535127b4373514f0c80fc631dd30b7b7
pd: 541e3a57cd996bc46e91def9eb5437b05b5da6b0
================================================================================
oltp_insert:
    * QPS: 4669.07 ± 0.36% (std=12.75) delta: -0.00% (p=0.987)
    * Latency p50: 27.41 ± 0.36% (std=0.07) delta: 0.01%
    * Latency p99: 44.60 ± 4.56% (std=1.34) delta: 0.93%
            
oltp_point_select:
    * QPS: 43083.57 ± 0.32% (std=96.11) delta: 2.10% (p=0.001)
    * Latency p50: 2.97 ± 0.22% (std=0.00) delta: -1.87%
    * Latency p99: 10.09 ± 0.00% (std=0.00) delta: -1.18%
            
oltp_read_write:
    * QPS: 14747.10 ± 0.66% (std=59.57) delta: -0.06% (p=0.819)
    * Latency p50: 173.89 ± 0.66% (std=0.71) delta: 0.07%
    * Latency p99: 235.74 ± 0.00% (std=0.00) delta: 0.00%
            
oltp_update_index:
    * QPS: 4223.51 ± 0.66% (std=16.94) delta: -0.17% (p=0.599)
    * Latency p50: 30.31 ± 0.67% (std=0.12) delta: 0.18%
    * Latency p99: 54.50 ± 1.20% (std=0.46) delta: 1.82%
            
oltp_update_non_index:
    * QPS: 4737.39 ± 0.23% (std=7.30) delta: 0.39% (p=0.314)
    * Latency p50: 27.02 ± 0.23% (std=0.04) delta: -0.39%
    * Latency p99: 42.06 ± 5.03% (std=1.26) delta: -1.79%
            

@djshow832
Copy link
Contributor Author

/run-all-tests

@djshow832 djshow832 force-pushed the stmt_summary_improve_concurrency branch from 0b5f79d to 2e8d43f Compare January 17, 2020 07:53
@djshow832
Copy link
Contributor Author

/run-all-tests

Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 added status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement. labels Jan 19, 2020
@bb7133 bb7133 added this to the v3.0.10 milestone Jan 19, 2020
@djshow832 djshow832 force-pushed the stmt_summary_improve_concurrency branch from ac86eeb to 5899162 Compare February 3, 2020 04:07
@djshow832
Copy link
Contributor Author

/run-all-tests


execDetail := stmtCtx.GetExecDetails()
copTaskInfo := stmtCtx.CopTasksDetails()
memMax := stmtCtx.MemTracker.MaxConsumed()
var userString string
if sessVars.User != nil {
userString = sessVars.User.String()
userString = sessVars.User.Username
Copy link
Contributor

Choose a reason for hiding this comment

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

Attention, User.Username doesn't contain the User.Hostname, are you sure about this change?

If User.String() is slow, how bout optimize the funtion, we can use string + string... instead of fmt.Sprintf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether host name helps when analyzing performance. According to your experience, is it helpful?

Copy link
Contributor

Choose a reason for hiding this comment

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

I rarely use use.host info.

@sre-bot
Copy link
Contributor

sre-bot commented Feb 3, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 3, 2020

@djshow832 merge failed.

@djshow832
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Feb 3, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 3, 2020

@djshow832 merge failed.

@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

/run-all-tests

@djshow832
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

/run-all-tests

@djshow832
Copy link
Contributor Author

/build

@djshow832
Copy link
Contributor Author

/rebuild

@djshow832
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

/run-all-tests

@sre-bot sre-bot merged commit 7ba002e into pingcap:master Feb 4, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 2192448e501757fd771b0a255b1265b510f637e5
+++ tidb: 2b6eba64a05c429e5b867f2833c93727c330f7e6
tikv: 0f08b6beafe17825329d4a1c00a22db6b6699e6c
pd: 4686d4622ad3ebc7fc267b5c64437129cee356fd
================================================================================
oltp_insert:
    * QPS: 4714.69 ± 0.43% (std=14.24) delta: 0.05% (p=0.847)
    * Latency p50: 27.15 ± 0.43% (std=0.08) delta: -0.05%
    * Latency p99: 47.09 ± 6.21% (std=2.19) delta: 1.42%
            
oltp_point_select:
    * QPS: 42531.46 ± 0.47% (std=138.74) delta: -0.36% (p=0.292)
    * Latency p50: 3.01 ± 0.58% (std=0.01) delta: 0.42%
    * Latency p99: 10.09 ± 0.00% (std=0.00) delta: -1.75%
            
oltp_read_write:
    * QPS: 15408.32 ± 0.45% (std=45.62) delta: 0.41% (p=0.194)
    * Latency p50: 166.50 ± 0.41% (std=0.48) delta: -0.38%
    * Latency p99: 327.49 ± 2.24% (std=4.87) delta: -1.78%
            
oltp_update_index:
    * QPS: 6261.06 ± 0.62% (std=23.69) delta: -0.09% (p=0.727)
    * Latency p50: 20.44 ± 0.62% (std=0.08) delta: 0.09%
    * Latency p99: 38.25 ± 1.80% (std=0.49) delta: 0.00%
            
oltp_update_non_index:
    * QPS: 4707.55 ± 0.33% (std=10.86) delta: 0.14% (p=0.332)
    * Latency p50: 27.19 ± 0.33% (std=0.06) delta: -0.11%
    * Latency p99: 41.60 ± 1.20% (std=0.35) delta: -0.60%
            

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement. type/performance type/usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants