infoschema: add 3 fields to statement summary table #14096

djshow832 · 2019-12-17T09:12:40Z

What problem does this PR solve?

Add 3 more fields to events_statements_summary_by_digest:

SUMMARY_END_TIME: the end time of current summary.
SUM_BACKOFF_TIMES: the total backoff times of this kind of SQL.
PREV_SAMPLE_TEXT: the previous SQL before current SQL if current SQL is commit.

What is changed and how it works?

SUMMARY_END_TIME: the end time will change if refreshInterval changes.
PREV_SAMPLE_TEXT: the identifier of each kind of SQL is composed of previous digest and current digest, so it can distinguish different types of transactions.

Check List

Tests

Unit test

Code changes

Has exported variable/fields change

Side effects

Possible performance regression
Increased code complexity

Related changes

Need to cherry-pick to the release branch
Need to update the documentation

Release note

Add 3 more fields to table events_statements_summary_by_digest:
SUMMARY_END_TIME: the end time of current summary.
SUM_BACKOFF_TIMES: the total backoff times of this kind of SQL.
PREV_SAMPLE_TEXT: the previous SQL before current SQL if current SQL is commit.

djshow832 · 2019-12-17T09:12:51Z

/bench

sre-bot · 2019-12-17T11:32:46Z

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 5940bb2ceb7de833839b09341356ef6b0fe38cb7
+++ tidb: fb4d8dbd48b01a9be297e938cc932d332a61ee5d
tikv: 7f17c967ff9d9d778b2c39432cc5465096187261
pd: 8198108644e65c08d6beb7e28ff54ddade5ec542
================================================================================
oltp_update_non_index:
    * QPS: 9263.07 ± 0.26% (std=17.85) delta: 0.39% (p=0.378)
    * Latency p50: 13.81 ± 0.25% (std=0.03) delta: -0.42%
    * Latency p99: 27.66 ± 0.00% (std=0.00) delta: 0.00%
            
oltp_insert:
    * QPS: 4709.87 ± 0.16% (std=5.40) delta: 0.28% (p=0.682)
    * Latency p50: 27.17 ± 0.16% (std=0.03) delta: -0.27%
    * Latency p99: 46.46 ± 5.92% (std=2.00) delta: -2.16%
            
oltp_read_write:
    * QPS: 16316.36 ± 0.12% (std=12.09) delta: 0.33% (p=0.422)
    * Latency p50: 157.27 ± 0.11% (std=0.10) delta: -0.30%
    * Latency p99: 316.39 ± 2.39% (std=5.34) delta: 1.22%
            
oltp_update_index:
    * QPS: 4245.43 ± 0.33% (std=10.60) delta: -0.31% (p=0.997)
    * Latency p50: 30.15 ± 0.35% (std=0.08) delta: 0.31%
    * Latency p99: 54.84 ± 3.56% (std=1.20) delta: -1.19%
            
oltp_point_select:
    * QPS: 41974.43 ± 0.14% (std=41.55) delta: -0.35% (p=0.709)
    * Latency p50: 3.05 ± 0.22% (std=0.00) delta: 0.22%
    * Latency p99: 9.91 ± 1.82% (std=0.13) delta: 0.00%

codecov · 2019-12-18T02:41:38Z

Codecov Report

❗ No coverage uploaded for pull request base (master@2cb5eb6). Click here to learn what that means.
The diff coverage is 83.6363%.

@@             Coverage Diff             @@
##             master     #14096   +/-   ##
===========================================
  Coverage          ?   80.1793%           
===========================================
  Files             ?        483           
  Lines             ?     121535           
  Branches          ?          0           
===========================================
  Hits              ?      97446           
  Misses            ?      16320           
  Partials          ?       7769

lonng · 2019-12-18T09:12:12Z

infoschema/perfschema/const.go


 // tableEventsStatementsSummaryByDigestHistory contains the column name definitions for table
 // events_statements_summary_by_digest_history.
 const tableEventsStatementsSummaryByDigestHistory = "CREATE TABLE if not exists events_statements_summary_by_digest_history (" +
 	"SUMMARY_BEGIN_TIME TIMESTAMP(6) NOT NULL," +
+	"SUMMARY_END_TIME TIMESTAMP(6) NOT NULL," +


We can use a constant format string to save the create table SQL if the digest and the history digest table have the same schema.

Good, done.

lonng · 2019-12-18T09:31:29Z

util/stmtsummary/statement_summary.go

 		key.hash = append(key.hash, hack.Slice(key.digest)...)
 		key.hash = append(key.hash, hack.Slice(key.schemaName)...)
+		key.hash = append(key.hash, hack.Slice(key.prevDigest)...)


The same SQL statement is executed after different statements, the final digest is different? Is it expected?

Actually, only when current SQL is commit do we record prevSQL. Otherwise, prevSQL is empty. See adaptor.go.
If all commits are grouped in one record, we can't distinguish different transactions.

@djshow832 OK, It's a convention of coding and it's better to add some comments to hint the maintainer don't break it.

lonng · 2019-12-18T09:53:40Z

util/stmtsummary/statement_summary_test.go

+	ssElement = ssbd.history.Back().Value.(*stmtSummaryByDigestElement)
+	c.Assert(ssElement.beginTime, GreaterEqual, now-60)
+	c.Assert(ssElement.beginTime, LessEqual, now2)
+	c.Assert(ssElement.endTime-ssElement.beginTime, Equals, int64(60))


Do we need to add some cases to test the expiration?

TestSummaryHistory tested expiration.

lonng

Reset LGTM

sessionctx/variable/session.go

bb7133 · 2019-12-18T11:57:42Z

Also, please update the Release Notes in your PR Description: list the name of 3 newly added fields and give a brief explanation on why they're added.

infoschema/perfschema/const.go

sessionctx/variable/session.go

bb7133 · 2019-12-19T13:01:20Z

LGTM

djshow832 · 2019-12-19T13:04:23Z

/run-unit-test

djshow832 · 2019-12-19T13:10:11Z

/run-all-tests

djshow832 added status/DNM type/usability labels Dec 17, 2019

djshow832 force-pushed the stmt_summary_end_time branch 2 times, most recently from 713e06f to c7a869b Compare December 18, 2019 02:41

djshow832 removed the status/DNM label Dec 18, 2019

djshow832 requested review from zimulala, AilinKid, bb7133, crazycs520 and lonng December 18, 2019 04:48

lonng reviewed Dec 18, 2019

View reviewed changes

djshow832 force-pushed the stmt_summary_end_time branch from c7a869b to c1d89fa Compare December 18, 2019 09:22

lonng reviewed Dec 18, 2019

View reviewed changes

bb7133 reviewed Dec 18, 2019

View reviewed changes

sessionctx/variable/session.go Outdated Show resolved Hide resolved

AilinKid reviewed Dec 18, 2019

View reviewed changes

infoschema/perfschema/const.go Show resolved Hide resolved

sessionctx/variable/session.go Show resolved Hide resolved

djshow832 force-pushed the stmt_summary_end_time branch from c1d89fa to d334aac Compare December 19, 2019 04:02

bb7133 added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 19, 2019

djshow832 added 5 commits December 19, 2019 20:54

add 3 fields to summary table

4af51ef

disable stmt summary

a2aca4e

simplify table fields definition

7d3b9bc

fix prevStmtDigest

1400ce5

add comments

729e4c4

djshow832 force-pushed the stmt_summary_end_time branch from d334aac to 729e4c4 Compare December 19, 2019 12:54

bb7133 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 19, 2019

bb7133 approved these changes Dec 19, 2019

View reviewed changes

Merge branch 'master' into stmt_summary_end_time

17ab60f

djshow832 merged commit fcc5dba into pingcap:master Dec 19, 2019

djshow832 added a commit to djshow832/tidb that referenced this pull request Dec 20, 2019

infoschema: add 3 fields to statement summary table (pingcap#14096)

49aedae

bb7133 pushed a commit that referenced this pull request Dec 20, 2019

infoschema: add 3 fields to statement summary table (#14096) (#14168)

b47381b

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

infoschema: add 3 fields to statement summary table (pingcap#14096)

560762f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

infoschema: add 3 fields to statement summary table #14096

infoschema: add 3 fields to statement summary table #14096

djshow832 commented Dec 17, 2019 •

edited

Loading

djshow832 commented Dec 17, 2019

sre-bot commented Dec 17, 2019

codecov bot commented Dec 18, 2019 •

edited

Loading

lonng Dec 18, 2019

djshow832 Dec 18, 2019

lonng Dec 18, 2019

djshow832 Dec 18, 2019

lonng Dec 18, 2019

djshow832 Dec 19, 2019

lonng Dec 18, 2019

djshow832 Dec 18, 2019

lonng left a comment

bb7133 commented Dec 18, 2019

bb7133 commented Dec 19, 2019

djshow832 commented Dec 19, 2019

djshow832 commented Dec 19, 2019

infoschema: add 3 fields to statement summary table #14096

infoschema: add 3 fields to statement summary table #14096

Conversation

djshow832 commented Dec 17, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

djshow832 commented Dec 17, 2019

sre-bot commented Dec 17, 2019

Benchmark Report

codecov bot commented Dec 18, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lonng left a comment

Choose a reason for hiding this comment

bb7133 commented Dec 18, 2019

bb7133 commented Dec 19, 2019

djshow832 commented Dec 19, 2019

djshow832 commented Dec 19, 2019

djshow832 commented Dec 17, 2019 •

edited

Loading

codecov bot commented Dec 18, 2019 •

edited

Loading