*: support read consistency isolation level in the pessimistic transactions #14087

jackysp · 2019-12-17T02:59:52Z

What problem does this PR solve?

TiDB does not support Oracle like read consistency isolation level.

What is changed and how it works?

Support read consistency isolation level in the pessimistic transactions.

Check List

Tests

Unit test

Code changes

Has exported function/method change

codecov · 2019-12-17T03:29:03Z

Codecov Report

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

@@             Coverage Diff             @@
##             master     #14087   +/-   ##
===========================================
  Coverage          ?   80.1464%           
===========================================
  Files             ?        483           
  Lines             ?     121530           
  Branches          ?          0           
===========================================
  Hits              ?      97402           
  Misses            ?      16363           
  Partials          ?       7765

jackysp · 2019-12-17T04:34:42Z

/run-all-tests

jackysp · 2019-12-17T05:33:32Z

/bench

sre-bot · 2019-12-17T06:57:34Z

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: a61e3cc324e800d54e5f70a6253eb027665b8a64
+++ tidb: 81cdde210aa8e0b7edd60050c8487e420b10ccce
tikv: 7f17c967ff9d9d778b2c39432cc5465096187261
pd: f3f89205f4a361d8f2116c780aeaab3777b7775b
================================================================================
oltp_update_non_index:
    * QPS: 4727.46 ± 0.15% (std=5.53) delta: -0.03% (p=0.727)
    * Latency p50: 27.08 ± 0.17% (std=0.03) delta: 0.04%
    * Latency p99: 40.86 ± 1.19% (std=0.34) delta: -0.59%
            
oltp_insert:
    * QPS: 4776.71 ± 0.04% (std=1.28) delta: -0.22% (p=0.041)
    * Latency p50: 26.79 ± 0.05% (std=0.01) delta: 0.22%
    * Latency p99: 46.35 ± 1.21% (std=0.40) delta: 2.50%
            
oltp_read_write:
    * QPS: 15137.10 ± 1.08% (std=96.51) delta: -0.40% (p=0.359)
    * Latency p50: 168.87 ± 0.19% (std=0.23) delta: -0.06%
    * Latency p99: 321.65 ± 2.24% (std=4.78) delta: 2.89%
            
oltp_update_index:
    * QPS: 4219.03 ± 0.42% (std=12.33) delta: -0.06% (p=0.809)
    * Latency p50: 30.33 ± 0.40% (std=0.09) delta: 0.03%
    * Latency p99: 54.50 ± 1.20% (std=0.46) delta: 1.82%
            
oltp_point_select:
    * QPS: 38725.96 ± 0.61% (std=155.61) delta: -0.33% (p=0.341)
    * Latency p50: 3.31 ± 0.45% (std=0.01) delta: 0.30%
    * Latency p99: 10.09 ± 1.78% (std=0.13) delta: 0.00%

jackysp · 2019-12-17T07:02:13Z

Version:
    tidb: 3a520a4edfb0d53fc0e8cb73ff87974bb887996c(HEAD) 2019-12-17 04:34:54
    tikv: 7f17c967ff9d9d778b2c39432cc5465096187261(master) 2019-12-17 04:47:36
    pd: f3f89205f4a361d8f2116c780aeaab3777b7775b(master) 2019-12-16 08:46:06
        
Compare:
    tidb: https://github.com/pingcap/tidb/compare/...3a520a4
    tikv: https://github.com/tikv/tikv/compare/...7f17c96
    pd: https://github.com/pingcap/pd/compare/...f3f8920
        
test-1: < oltp_update_non_index >
    * QPS : 33176.75 ± 1.9187% (std=391.66) delta: 0.00% (p=0.000)
    * TPS : 33176.75 ± 1.9187% (std=391.66) delta: 0.00%
    * AvgMs : 7.71 ± 1.8927% (std=0.09) delta: 0.00%
    * PercentileMs99 : 20.59 ± 2.5155% (std=0.30) delta: 0.00%
            
test-2: < oltp_insert >
    * QPS : 23801.43 ± 0.0372% (std=7.15) delta: 0.00% (p=0.000)
    * TPS : 23801.43 ± 0.0372% (std=7.15) delta: 0.00%
    * AvgMs : 10.75 ± 0.0000% (std=0.00) delta: 0.00%
    * PercentileMs99 : 21.89 ± 0.0000% (std=0.00) delta: 0.00%
            
test-3: < oltp_read_write >
    * QPS : 39764.17 ± 0.2546% (std=89.28) delta: 0.00% (p=0.000)
    * TPS : 1988.86 ± 0.2540% (std=4.43) delta: 0.00%
    * AvgMs : 128.64 ± 0.2565% (std=0.28) delta: 0.00%
    * PercentileMs99 : 178.01 ± 1.0819% (std=1.57) delta: 0.00%
            
test-4: < oltp_write_only >
    * QPS : 35654.11 ± 0.7277% (std=178.34) delta: 0.00% (p=0.000)
    * TPS : 5942.95 ± 0.7275% (std=29.71) delta: 0.00%
    * AvgMs : 43.07 ± 0.7291% (std=0.21) delta: 0.00%
    * PercentileMs99 : 77.19 ± 0.0000% (std=0.00) delta: 0.00%
            
test-5: < oltp_update_index >
    * QPS : 20458.00 ± 1.0394% (std=144.52) delta: 0.00% (p=0.000)
    * TPS : 20458.00 ± 1.0394% (std=144.52) delta: 0.00%
    * AvgMs : 12.44 ± 1.0775% (std=0.10) delta: 0.00%
    * PercentileMs99 : 28.98 ± 1.0765% (std=0.25) delta: 0.00%

pessimistic repeatable-read from @mahjonp

jackysp · 2019-12-17T10:59:05Z

Repeatable Read vs. Read Comitted by this PR,

Version:
   tidb: 3a520a4edfb0d53fc0e8cb73ff87974bb887996c(HEAD) 2019-12-17 04:34:54
   tikv: 7f17c967ff9d9d778b2c39432cc5465096187261(master) 2019-12-17 04:47:36
   pd: f3f89205f4a361d8f2116c780aeaab3777b7775b(master) 2019-12-16 08:46:06
       
Compare:
   tidb: https://github.com/pingcap/tidb/compare/3a520a4...3a520a4
   tikv: https://github.com/tikv/tikv/compare/7f17c96...7f17c96
   pd: https://github.com/pingcap/pd/compare/f3f8920...f3f8920
       
test-1: < oltp_update_non_index >
   * QPS : 33206.23 ± 0.7837% (std=165.70) delta: 0.09% (p=0.895)
   * TPS : 33206.23 ± 0.7837% (std=165.70) delta: 0.09%
   * AvgMs : 7.71 ± 0.8044% (std=0.04) delta: -0.08%
   * PercentileMs99 : 20.74 ± 0.0000% (std=0.00) delta: 0.72%
           
test-2: < oltp_insert >
   * QPS : 23588.83 ± 1.5003% (std=213.26) delta: -0.89% (p=0.106)
   * TPS : 23588.83 ± 1.5003% (std=213.26) delta: -0.89%
   * AvgMs : 10.85 ± 1.4934% (std=0.10) delta: 0.91%
   * PercentileMs99 : 22.09 ± 0.8830% (std=0.20) delta: 0.89%
           
test-3: < oltp_read_write >
   * QPS : 38889.13 ± 0.2850% (std=81.31) delta: -2.20% (p=0.001)
   * TPS : 1945.06 ± 0.2832% (std=4.05) delta: -2.20%
   * AvgMs : 131.52 ± 0.2844% (std=0.26) delta: 2.24%
   * PercentileMs99 : 178.01 ± 1.0819% (std=1.57) delta: 0.00%
           
test-4: < oltp_write_only >
   * QPS : 35770.84 ± 0.6482% (std=167.74) delta: 0.33% (p=0.368)
   * TPS : 5962.38 ± 0.6488% (std=27.95) delta: 0.33%
   * AvgMs : 42.92 ± 0.6570% (std=0.20) delta: -0.33%
   * PercentileMs99 : 76.37 ± 1.0764% (std=0.67) delta: -1.06%
           
test-5: < oltp_update_index >
   * QPS : 20283.99 ± 2.1011% (std=256.59) delta: -0.85% (p=0.280)
   * TPS : 20283.99 ± 2.1011% (std=256.59) delta: -0.85%
   * AvgMs : 12.55 ± 0.2788% (std=0.03) delta: 0.96%
   * PercentileMs99 : 29.19 ± 0.0000% (std=0.00) delta: 0.72%

executor/adapter.go

accident

sessionctx/variable/session.go

executor/builder.go

coocood · 2019-12-18T06:46:11Z

LGTM

sessionctx/variable/session.go

session/pessimistic_test.go

executor/builder.go

jackysp · 2019-12-18T12:52:57Z

/run-all-tests

jackysp · 2019-12-19T02:18:35Z

/run-all-tests

lysu

rest LGTM, but it seems wasted to get ts for each read in TiDB (even if no any data changed)

it seems we just need "read newest data" instead of "read current ts data", maybe we can optimize this base on https://github.com/tikv/tikv/pull/6123/files#diff-49763d722eeb4620a599e6bddecf8f56R50 ?

send a NaN ts and mark it's RC read , and kv mantain max read/write ts watermark like this ts_cache in each region leader, then kv replace NaN with max ts in ts_cache to read newest data, and piggyback ts to tidb.... seems be better?

~~if memory ts_cache is crashed, kv can report a retry error, tidb retake current ts and resend~~

~~maybe we can do it later~~

executor/adapter.go

executor/builder.go

planner/optimize.go

jackysp · 2019-12-20T04:56:37Z

We could not know if the data changed or not before read them. Maybe I could avoid the get ts requests of each statement of RR. @lysu
What do you think? @coocood

coocood · 2019-12-20T07:21:42Z

LGTM

jackysp · 2019-12-23T05:51:59Z

/run-all-tests tidb-test=pr/971

jackysp · 2019-12-23T07:16:09Z

PTAL @lysu @cfzjywxk

session/isolation_test.go

tiancaiamao · 2019-12-23T09:02:56Z

session/isolation_test.go

+
+	session1.MustExec("begin;")
+	session2.MustExec("begin;")
+	session1.MustQuery("select c from x where id = 1;").Check(testkit.Rows("1"))


Use select for update and mysql should be no write skew here?

tiancaiamao · 2019-12-23T09:10:22Z

session/isolation_test.go

+	session2.MustExec("insert into x values(2, 1);")
+	session2.MustExec("commit;")
+	session1.MustExec("update x set c = c+1 where id < 5;")
+	session1.MustQuery("select c from x where id < 5;").Check(testkit.Rows("2", "2"))


Compatible for MySQL's drawback -_-||
This PhantomRead is a designed feature!

tiancaiamao · 2019-12-23T10:12:47Z

sessionctx/variable/session.go

+		return s.TxnCtx.Isolation == ast.ReadCommitted
+	}
+	if s.txnIsolationLevelOneShot.state == oneShotUse {
+		s.TxnCtx.Isolation = s.txnIsolationLevelOneShot.value


When will this value be reset?

When the session starts a new transaction, it will create a new TxnCtx. It works like StmtCtx for each statement.

cfzjywxk

LGTM

session/isolation_test.go

cfzjywxk · 2019-12-23T12:31:33Z

session/isolation_test.go

+	session2.MustExec("begin;")
+	session1.MustQuery("select c from x where id = 1;").Check(testkit.Rows("1"))
+	session2.MustQuery("select c from y where id = 1;").Check(testkit.Rows("1"))
+	session1.MustExec("update y set c = c+1 where id = 1;")


Should it be like this?

x.c1 <- 1 y.c1 <- 2 session1 session2 begin begin x.c1 <- y.c1 y.c1 <- x.c1 commit commit

and verify x.c1 and y.c1 get swapped but not the same

Signed-off-by: Shuaipeng Yu <[email protected]>

jackysp · 2019-12-24T03:32:58Z

PTAL @cfzjywxk @tiancaiamao

cfzjywxk · 2019-12-24T11:01:40Z

LGTM

sre-bot · 2019-12-24T13:20:41Z

/run-all-tests

jackysp added type/new-feature sig/transaction SIG:Transaction labels Dec 17, 2019

jackysp requested a review from a team as a code owner December 17, 2019 02:59

ghost requested review from eurekaka and francis0407 and removed request for a team December 17, 2019 03:00

eurekaka removed their request for review December 17, 2019 03:03

jackysp added the status/WIP label Dec 17, 2019

jackysp force-pushed the read_consistency branch from a1e5e22 to 3a520a4 Compare December 17, 2019 04:34

jackysp added the status/all tests passed label Dec 17, 2019

jackysp removed the status/WIP label Dec 17, 2019

jackysp requested review from coocood, tiancaiamao, cfzjywxk and lysu December 17, 2019 10:59

coocood previously approved these changes Dec 18, 2019

View reviewed changes

executor/adapter.go Outdated Show resolved Hide resolved

coocood reviewed Dec 18, 2019

View reviewed changes

sessionctx/variable/session.go Show resolved Hide resolved

coocood reviewed Dec 18, 2019

View reviewed changes

executor/builder.go Outdated Show resolved Hide resolved

cfzjywxk reviewed Dec 18, 2019

View reviewed changes

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

session/pessimistic_test.go Show resolved Hide resolved

executor/builder.go Outdated Show resolved Hide resolved

jackysp force-pushed the read_consistency branch from c7a32c3 to ba18d19 Compare December 19, 2019 02:18

lysu reviewed Dec 19, 2019

View reviewed changes

executor/adapter.go Show resolved Hide resolved

executor/builder.go Outdated Show resolved Hide resolved

planner/optimize.go Show resolved Hide resolved

jackysp force-pushed the read_consistency branch from 2b39b57 to d48fcdd Compare December 23, 2019 03:01

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

tiancaiamao reviewed Dec 23, 2019

View reviewed changes

cfzjywxk reviewed Dec 23, 2019

View reviewed changes

jackysp force-pushed the read_consistency branch 3 times, most recently from f6616fb to 349cee9 Compare December 24, 2019 03:32

*: support read consistency isolation level

349cee9

Signed-off-by: Shuaipeng Yu <[email protected]>

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

cfzjywxk added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 24, 2019

lysu approved these changes Dec 24, 2019

View reviewed changes

lysu added the status/can-merge Indicates a PR has been approved by a committer. label Dec 24, 2019

Merge branch 'master' into read_consistency

bfc3a31

sre-bot merged commit 1335ffb into pingcap:master Dec 24, 2019

jackysp deleted the read_consistency branch February 27, 2020 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support read consistency isolation level in the pessimistic transactions #14087

*: support read consistency isolation level in the pessimistic transactions #14087

jackysp commented Dec 17, 2019

codecov bot commented Dec 17, 2019 •

edited

Loading

jackysp commented Dec 17, 2019

jackysp commented Dec 17, 2019

sre-bot commented Dec 17, 2019

jackysp commented Dec 17, 2019

jackysp commented Dec 17, 2019

coocood commented Dec 18, 2019

jackysp commented Dec 18, 2019

jackysp commented Dec 19, 2019

lysu left a comment •

edited

Loading

jackysp commented Dec 20, 2019

coocood commented Dec 20, 2019

jackysp commented Dec 23, 2019

jackysp commented Dec 23, 2019

tiancaiamao Dec 23, 2019

jackysp Dec 23, 2019

tiancaiamao Dec 23, 2019

tiancaiamao Dec 23, 2019

jackysp Dec 23, 2019

cfzjywxk left a comment

cfzjywxk Dec 23, 2019

jackysp Dec 24, 2019

jackysp commented Dec 24, 2019

cfzjywxk commented Dec 24, 2019

sre-bot commented Dec 24, 2019

*: support read consistency isolation level in the pessimistic transactions #14087

*: support read consistency isolation level in the pessimistic transactions #14087

Conversation

jackysp commented Dec 17, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Dec 17, 2019 • edited Loading

Codecov Report

jackysp commented Dec 17, 2019

jackysp commented Dec 17, 2019

sre-bot commented Dec 17, 2019

Benchmark Report

jackysp commented Dec 17, 2019

jackysp commented Dec 17, 2019

coocood commented Dec 18, 2019

jackysp commented Dec 18, 2019

jackysp commented Dec 19, 2019

lysu left a comment • edited Loading

Choose a reason for hiding this comment

jackysp commented Dec 20, 2019

coocood commented Dec 20, 2019

jackysp commented Dec 23, 2019

jackysp commented Dec 23, 2019

tiancaiamao Dec 23, 2019

Choose a reason for hiding this comment

jackysp Dec 23, 2019

Choose a reason for hiding this comment

tiancaiamao Dec 23, 2019

Choose a reason for hiding this comment

tiancaiamao Dec 23, 2019

Choose a reason for hiding this comment

jackysp Dec 23, 2019

Choose a reason for hiding this comment

cfzjywxk left a comment

Choose a reason for hiding this comment

cfzjywxk Dec 23, 2019

Choose a reason for hiding this comment

jackysp Dec 24, 2019

Choose a reason for hiding this comment

jackysp commented Dec 24, 2019

cfzjywxk commented Dec 24, 2019

sre-bot commented Dec 24, 2019

codecov bot commented Dec 17, 2019 •

edited

Loading

lysu left a comment •

edited

Loading