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

ddl: ddl-owner try to use memory infoSchema to check first #10170

Merged
merged 12 commits into from
Apr 23, 2019

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Apr 16, 2019

What problem does this PR solve?

Make ddl-owner try to use memory infoschema to check first if the owner tidb-server already has loaded the latest schema.

Here is a simple test, There are 11394 tables in test20.
Deploy 1 tidb, 1 pd, 1 tikv.

create table

mysql root@localhost:test20> show tables;
11394 rows in set
Time: 0.253s

Before, spend 0.37s, checkTableNotExists spend 0.34s

mysql root@localhost:test20> create table tb (a int);
Query OK, 0 rows affected
Time: 0.371s 

In this PR: spend 0.07s, checkTableNotExists spend 267us.

mysql root@localhost:test20> create table ta (a int);
Query OK, 0 rows affected
Time: 0.070s

Create database

There are 18500 databases already in TiDB.

mysql root@localhost:test> show databases;
18500 rows in set
Time: 0.546s

Before: spend 0.32S, and checkSchemaExist spend 250ms.

mysql root@localhost:test> create database dbd4
Query OK, 0 rows affected
Time: 0.326s

In this PR: 0.08s, checkSchemaExist spend 1 ms.

mysql root@localhost:test> create database dbd2
Query OK, 0 rows affected
Time: 0.080s

What is changed and how it works?

Change checkTableNotExists function.

Check List

Tests

  • No test...

Code changes

  • Has exported function/method change

Related changes

  • Need to cherry-pick to the release branch ?

@crazycs520 crazycs520 added component/DDL-need-LGT3 type/enhancement The issue or PR belongs to an enhancement. labels Apr 16, 2019
@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #10170 into master will increase coverage by 0.0167%.
The diff coverage is 77.4647%.

@@               Coverage Diff                @@
##             master     #10170        +/-   ##
================================================
+ Coverage   77.9363%   77.9531%   +0.0167%     
================================================
  Files           407        407                
  Lines         83463      83531        +68     
================================================
+ Hits          65048      65115        +67     
+ Misses        13588      13584         -4     
- Partials       4827       4832         +5

@crazycs520
Copy link
Contributor Author

/run-all-tests

ddl/table.go Show resolved Hide resolved
ddl/table.go Show resolved Hide resolved
Copy link
Contributor

@winkyao winkyao 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
Copy link
Member

bb7133 commented Apr 18, 2019

LGTM

@ngaut ngaut added status/LGT2 Indicates that a PR has LGTM 2. and removed component/DDL-need-LGT3 labels Apr 19, 2019
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Apr 23, 2019
@tiancaiamao tiancaiamao merged commit 95b66e4 into pingcap:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT3 The PR has already had 3 LGTM. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants