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

*: use new row-format in tidb #12634

Merged
merged 8 commits into from
Jan 2, 2020
Merged

*: use new row-format in tidb #12634

merged 8 commits into from
Jan 2, 2020

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Oct 11, 2019

What problem does this PR solve?

using https://github.com/pingcap/tidb/blob/master/docs/design/2018-07-19-row-format.md format in TiDB

follow up #7597 but get optimize for rowcodec from unionstore

tikv pr in tikv/tikv#6132

What is changed and how it works?

WIP... need more tunning to make performance can be improved(now testing with unionstore).

Check List

Tests

  • Unit test
  • Integration test(only integrate with unionstore or mocktikv, tikv wait development)
  • Manual test
Upgrade from old mocktikv or old unionstore to new format one, old table keep works
  • Performance test WIP

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Breaking backward compatibility
  • Need take care upgrade step

Related changes

  • n/a

Release note

  • use new-row format in tidb

This change is Reviewable

@lysu lysu added type/enhancement The issue or PR belongs to an enhancement. component/server status/WIP labels Oct 11, 2019
@lysu lysu changed the title *: use new-row format in tidb *: use new row-format in tidb Oct 11, 2019
@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@85afb82). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12634   +/-   ##
===========================================
  Coverage          ?   80.6571%           
===========================================
  Files             ?        483           
  Lines             ?     124325           
  Branches          ?          0           
===========================================
  Hits              ?     100277           
  Misses            ?      16121           
  Partials          ?       7927

@lysu
Copy link
Contributor Author

lysu commented Nov 18, 2019

read performance by https://github.com/lysu/sysbench/tree/dev-add-garbage, using --garbage_columns=170 table's point-get.

before:

sysbench --mysql-host=0.0.0.0 --mysql-port=4000 --mysql-user=root --threads=10 --tables=10 --table_size=30000 --time=60 --garbage_columns=170  oltp_point_select.lua run   
sysbench 1.1.0-1327e79 (using bundled LuaJIT 2.1.0-beta3)

Running the test with following options:
Number of threads: 10
Initializing random number generator from current time


Initializing worker threads...

Threads started!

SQL statistics:
    queries performed:
        read:                            673543
        write:                           0
        other:                           0
        total:                           673543
    transactions:                        673543 (11225.39 per sec.)
    queries:                             673543 (11225.39 per sec.)
    ignored errors:                      0      (0.00 per sec.)
    reconnects:                          0      (0.00 per sec.)

Throughput:
    events/s (eps):                      11225.3897
    time elapsed:                        60.0017s
    total number of events:              673543

Latency (ms):
         min:                                    0.15
         avg:                                    0.89
         max:                                   24.81
         95th percentile:                        2.14
         sum:                               599309.85

Threads fairness:
    events (avg/stddev):           67354.3000/84.41
    execution time (avg/stddev):   59.9310/0.00

after:

sysbench --mysql-host=0.0.0.0 --mysql-port=4000 --mysql-user=root --threads=10 --tables=10 --table_size=30000 --time=60 --garbage_columns=170  oltp_point_select.lua run   
sysbench 1.1.0-1327e79 (using bundled LuaJIT 2.1.0-beta3)

Running the test with following options:
Number of threads: 10
Initializing random number generator from current time


Initializing worker threads...

Threads started!

SQL statistics:
    queries performed:
        read:                            1112101
        write:                           0
        other:                           0
        total:                           1112101
    transactions:                        1112101 (18534.51 per sec.)
    queries:                             1112101 (18534.51 per sec.)
    ignored errors:                      0      (0.00 per sec.)
    reconnects:                          0      (0.00 per sec.)

Throughput:
    events/s (eps):                      18534.5106
    time elapsed:                        60.0016s
    total number of events:              1112101

Latency (ms):
         min:                                    0.11
         avg:                                    0.54
         max:                                   22.16
         95th percentile:                        1.06
         sum:                               599051.41

Threads fairness:
    events (avg/stddev):           111210.1000/92.84
    execution time (avg/stddev):   59.9051/0.00

/cc @coocood @jackysp

@coocood
Copy link
Member

coocood commented Nov 19, 2019

@lysu
how about select_random_ranges.lua workload?

@lysu
Copy link
Contributor Author

lysu commented Dec 17, 2019

/run-all-tests

@zhangjinpeng87
Copy link
Contributor

Amazing

@lysu
Copy link
Contributor Author

lysu commented Dec 24, 2019

/run-all-tests tikv=pr/6323

@lysu
Copy link
Contributor Author

lysu commented Dec 24, 2019

/run-integration-common-test tikv=pr/6323

@breezewish
Copy link
Member

/run-all-tests tikv=pr/6323

1 similar comment
@lysu
Copy link
Contributor Author

lysu commented Dec 24, 2019

/run-all-tests tikv=pr/6323

@lysu lysu marked this pull request as ready for review December 24, 2019 09:42
@lysu lysu removed the status/WIP label Dec 24, 2019
@lysu
Copy link
Contributor Author

lysu commented Dec 24, 2019

/run-all-tests tikv=pr/6323

util/codec/codec.go Outdated Show resolved Hide resolved
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
func (b *executorBuilder) buildBatchPointGet(plan *plannercore.BatchPointGetPlan) Executor {
startTS, err := b.getStartTS()
if err != nil {
b.err = err
return nil
}
decoder := newRowDecoder(b.ctx, plan.Schema(), plan.TblInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reuse the Decoder for the same table?

Copy link
Contributor Author

@lysu lysu Dec 25, 2019

Choose a reason for hiding this comment

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

our PointGetExecutor is pointget + projection , so plan.Schema() will get different results for select a from t where id = 1 and select a, b from t where id = 1(len1 vs len2), in previous commits, I have try to reuse decoder in cached plan level, but after discuss with others, it introduce some complex code, maybe we can renew at here but do it later?

tablecodec/tablecodec.go Outdated Show resolved Hide resolved
@lysu
Copy link
Contributor Author

lysu commented Dec 25, 2019

/run-all-tests

@lysu
Copy link
Contributor Author

lysu commented Dec 25, 2019

/run-all-tests tikv=pr/6323

@lysu
Copy link
Contributor Author

lysu commented Dec 25, 2019

/run-all-tests tikv=pr/6132

@lysu
Copy link
Contributor Author

lysu commented Jan 2, 2020

/run-all-tests

@lysu
Copy link
Contributor Author

lysu commented Jan 2, 2020

/bench

@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2020

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: b1c08ee2765ea6b370555dc172bb3b998a8cc6c0
+++ tidb: d4eae20aa781e13f6872556c50607606cfa70bf2
--- tikv: 267a5ebf7edb229cc53f5a8faa71975225bc6d8e
+++ tikv: 0de6ef5af5b75cf523ae0c54d9b433f62a407f06
pd: ea974c9488b8d304a72916d8cc50962f5ab99c5d
================================================================================
oltp_update_index:
    * QPS: 6404.59 ± 0.64% (std=26.52) delta: 0.25% (p=0.477)
    * Latency p50: 19.98 ± 0.64% (std=0.08) delta: -0.26%
    * Latency p99: 37.23 ± 0.90% (std=0.34) delta: -0.89%
            
oltp_insert:
    * QPS: 4769.73 ± 0.18% (std=6.40) delta: -0.04% (p=0.775)
    * Latency p50: 26.83 ± 0.19% (std=0.04) delta: 0.04%
    * Latency p99: 46.84 ± 2.24% (std=0.70) delta: -0.58%
            
oltp_read_write:
    * QPS: 16117.26 ± 0.02% (std=2.28) delta: 0.36% (p=0.276)
    * Latency p50: 159.14 ± 0.02% (std=0.02) delta: -0.41%
    * Latency p99: 301.53 ± 1.20% (std=2.55) delta: -2.42%
            
oltp_point_select:
    * QPS: 43379.19 ± 0.15% (std=38.50) delta: 1.49% (p=0.022)
    * Latency p50: 2.95 ± 0.00% (std=0.00) delta: -1.50%
    * Latency p99: 9.73 ± 0.00% (std=0.00) delta: 1.78%
            
oltp_update_non_index:
    * QPS: 4714.59 ± 0.02% (std=0.65) delta: -0.01% (p=0.906)
    * Latency p50: 27.15 ± 0.02% (std=0.00) delta: 0.00%
    * Latency p99: 41.10 ± 0.00% (std=0.00) delta: 0.00%
            

@glorv
Copy link
Contributor

glorv commented Jan 2, 2020

/run-unit-test

@lysu
Copy link
Contributor Author

lysu commented Jan 2, 2020

/bench +tpcc

workload:
  prepare: true

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 2, 2020
@coocood
Copy link
Member

coocood commented Jan 2, 2020

LGTM

@coocood
Copy link
Member

coocood commented Jan 2, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 2, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2020

Your auto merge job has been accepted, waiting for 14321

@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2020

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants