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

*: modify autoid allocator's Alloc() method, add context and tracing #21617

Merged
merged 6 commits into from Dec 10, 2020

Conversation

@tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Dec 9, 2020

What problem does this PR solve?

Problem Summary:

We observed a puzzling phenomenon that running the replace into ... statement result in transaction conflict on the autoid meta key.
I want to know why replace into ... triggers the auto-id allocation, but the trace result does not tell me why.

create table t (a varchar(20) primary key, b int, unique idx(b));
insert into t values ('1', 1);
replace into t values ('1', 2);  // autoid allocating here, weird, aha?!

After this change:

mysql> trace format = 'row' replace into t values ('1', 15);
+---------------------------------------------------------+-----------------+------------+
| operation                                               | startTS         | duration   |
+---------------------------------------------------------+-----------------+------------+
| trace                                                   | 22:21:48.086486 | 3.564142ms |
|   └─session.Execute                                     | 22:21:48.086494 | 3.516028ms |
|     ├─session.ParseSQL                                  | 22:21:48.086502 | 31.549µs   |
|     └─session.ExecuteStmt                               | 22:21:48.086583 | 3.407812ms |
|       ├─executor.Compile                                | 22:21:48.086593 | 149.667µs  |
|       └─session.runStmt                                 | 22:21:48.086767 | 3.160293ms |
|         ├─executor.handleNoDelayExecutor                | 22:21:48.086814 | 2.11548ms  |
|         │ └─*executor.ReplaceExec.Next                  | 22:21:48.086820 | 2.07282ms  |
|         │   ├─prefetchDataCache                         | 22:21:48.086881 | 256.207µs  |
|         │   │ ├─prefetchUniqueIndices                   | 22:21:48.086884 | 150.224µs  |
|         │   │ │ └─tikvTxn.BatchGet                      | 22:21:48.086887 | 132.829µs  |
|         │   │ │   └─regionRequest.SendReqCtx            | 22:21:48.086914 | 64.174µs   |
|         │   │ └─prefetchConflictedOldRows               | 22:21:48.087049 | 76.601µs   |
|         │   │   └─tikvTxn.BatchGet                      | 22:21:48.087055 | 58.811µs   |
|         │   │     └─regionRequest.SendReqCtx            | 22:21:48.087067 | 21.091µs   |
|         │   ├─table.AddRecord                           | 22:21:48.087266 | 1.596705ms |
|         │   │ └─autoid.Alloc                            | 22:21:48.087306 | 1.417151ms |
|         │   ├─index.Create                              | 22:21:48.088796 | 6.034µs    |
|         │   └─index.Create                              | 22:21:48.088829 | 3.042µs    |
|         └─session.CommitTxn                             | 22:21:48.088960 | 903.456µs  |
|           └─session.doCommitWitRetry                    | 22:21:48.088966 | 873.292µs  |
|             └─tikvTxn.Commit                            | 22:21:48.088979 | 778.94µs   |
|               ├─twoPhaseCommitter.prewriteMutations     | 22:21:48.089001 | 209.04µs   |
|               │ └─regionRequest.SendReqCtx              | 22:21:48.089036 | 129.653µs  |
|               ├─tikvStore.getTimestampWithRetry         | 22:21:48.089230 | 6.426µs    |
|               └─twoPhaseCommitter.commitMutations       | 22:21:48.089266 | 446.894µs  |
|                 └─regionRequest.SendReqCtx              | 22:21:48.089286 | 285.675µs  |
+---------------------------------------------------------+-----------------+------------+
27 rows in set (0.01 sec)

Get auto-id is bad for the performance of the replace statement:

  1. Every replace operation needs to get auto-id, and this takes 1ms+.
  2. Running replace operation in different sessions, the autoid.Alloc() transaction would conflict and make the situation worse.

We are not aware of it before.

B.T.W, the clustered index avoids this auto-id allocation, so the performance is much better.

What is changed and how it works?

Every operation involved with networking should be traced.

What's Changed:

Change the interface method, add the ctx parameter.

How it Works:

Trace the auto-id allocation.

Release note

  • Add tracing stub code to the table row ID allocation.
@tiancaiamao tiancaiamao requested a review from pingcap/exec-reviewers as a code owner Dec 9, 2020
@tiancaiamao tiancaiamao requested review from qw4990 and removed request for pingcap/exec-reviewers Dec 9, 2020
@tiancaiamao tiancaiamao requested a review from AilinKid Dec 9, 2020
tiancaiamao added 2 commits Dec 9, 2020
@AilinKid AilinKid requested a review from tangenta Dec 10, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

LGTM

Copy link
Contributor

@tangenta tangenta left a comment

LGTM

@ti-srebot ti-srebot removed the status/LGT1 label Dec 10, 2020
@tangenta
Copy link
Contributor

@tangenta tangenta commented Dec 10, 2020

/merge

@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

@tangenta Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: execution(slack).

@tiancaiamao
Copy link
Contributor Author

@tiancaiamao tiancaiamao commented Dec 10, 2020

/merge

@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

Your auto merge job has been accepted, waiting for:

  • 21533
@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

/run-all-tests

@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

@tiancaiamao merge failed.

@tiancaiamao
Copy link
Contributor Author

@tiancaiamao tiancaiamao commented Dec 10, 2020

/run-check_dev_2

@tiancaiamao
Copy link
Contributor Author

@tiancaiamao tiancaiamao commented Dec 10, 2020

/merge

@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

Your auto merge job has been accepted, waiting for:

  • 20747
@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

/run-all-tests

@ti-srebot ti-srebot merged commit 2905b0d into pingcap:master Dec 10, 2020
17 checks passed
17 checks passed
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_release_note Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_title Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/e2e Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-copr-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/tics-test job succeeded
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
@tiancaiamao tiancaiamao deleted the tiancaiamao:trace-alloc-id branch Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants