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

executor: improve trace format='row' #9029

Merged
merged 3 commits into from
Jan 13, 2019
Merged

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

In #8357, the trace result is in json format, used by web UI.

This commit add a row format for it, which look like this:

mysql> trace format='row' select * from t1 order by c limit 3;
+---------------------------+---------------------------+------------+
| operation                 | startTS                   | duration   |
+---------------------------+---------------------------+------------+
| session.getTxnFuture      | Jan 11 21:33:09.933212243 | 4.181µs    |
|   ├─session.Execute       | Jan 11 21:33:09.933207819 | 763.708µs  |
|   ├─session.ParseSQL      | Jan 11 21:33:09.933293211 | 57.002µs   |
|   ├─executor.Compile      | Jan 11 21:33:09.933388076 | 233.427µs  |
|   ├─session.runStmt       | Jan 11 21:33:09.933658842 | 198.985µs  |
|   ├─session.CommitTxn     | Jan 11 21:33:09.933808415 | 11.947µs   |
|   ├─recordSet.Next        | Jan 11 21:33:09.934038447 | 691.335µs  |
|   ├─topN.Next             | Jan 11 21:33:09.934042362 | 663.15µs   |
|   ├─tableReader.Next      | Jan 11 21:33:09.934050669 | 548.271µs  |
|   ├─tableReader.Next      | Jan 11 21:33:09.934671187 | 5µs        |
|   ├─recordSet.Next        | Jan 11 21:33:09.934756161 | 22.416µs   |
|   └─topN.Next             | Jan 11 21:33:09.934760051 | 1.553µs    |
+---------------------------+---------------------------+------------+
12 rows in set (0.00 sec)

What is changed and how it works?

Write the trace result to chunk in row format.

There are some small pitfalls I'll investigate later:

  1. The opentracing span parent-children relationship seems incorrect
  2. Add more columns to provide useful info

We support trace format='json' ... and trace format='row' ...
Maybe I should also make format=row the default one.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to update the documentation

@tiancaiamao
Copy link
Contributor Author

PTAL @jackysp @lysu

mysql> trace format='row' select * from trace where id = 0;
+---------------------------+-----------------+------------+
| operation                 | startTS         | duration   |
+---------------------------+-----------------+------------+
| session.getTxnFuture      | 22:08:38.247834 | 78.909µs   |
|   ├─session.Execute       | 22:08:38.247829 | 1.478487ms |
|   ├─session.ParseSQL      | 22:08:38.248457 | 71.159µs   |
|   ├─executor.Compile      | 22:08:38.248578 | 45.329µs   |
|   ├─session.runStmt       | 22:08:38.248661 | 75.13µs    |
|   ├─session.CommitTxn     | 22:08:38.248699 | 13.213µs   |
|   └─recordSet.Next        | 22:08:38.249340 | 155.317µs  |
+---------------------------+-----------------+------------+
7 rows in set (0.01 sec)
@codecov-io
Copy link

Codecov Report

Merging #9029 into master will increase coverage by 0.05%.
The diff coverage is 77.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9029      +/-   ##
==========================================
+ Coverage   67.58%   67.63%   +0.05%     
==========================================
  Files         364      364              
  Lines       75477    75471       -6     
==========================================
+ Hits        51011    51047      +36     
+ Misses      19973    19927      -46     
- Partials     4493     4497       +4
Impacted Files Coverage Δ
executor/trace.go 74.15% <77.04%> (+45.73%) ⬆️
ddl/session_pool.go 82.75% <0%> (-10.35%) ⬇️
ddl/delete_range.go 77.14% <0%> (-1.72%) ⬇️
store/tikv/lock_resolver.go 41.7% <0%> (-0.95%) ⬇️
expression/schema.go 94.95% <0%> (+0.84%) ⬆️
executor/join.go 78.96% <0%> (+1.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 504b10c...59e0d0f. Read the comment docs.

@jackysp
Copy link
Member

jackysp commented Jan 12, 2019

/run-all-tests

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
PTAL @lysu

}

chk.AppendString(0, prefix+suffix+t.Span.Name())
chk.AppendString(1, start.Format("15:04:05.000000"))
Copy link
Member

Choose a reason for hiding this comment

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

It is OK for me, but "15:04:05.000000" is still a little strange.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This control the time format output.
Go use pattern match to control the output format, https://godoc.org/time#Parse
And the layout string always use this date: 2006 01 02 15 04 05, it's a special number

if you want the result looks like "hh:mm:ss.xxxxxx" , the layout should be "15:04:05.000000"

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

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

@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

/run-all-tests

@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

LGTM to current code, but maybe more refine the span name in the next @tiancaiamao

mysql> trace format="row" select * from t join t2 on t2.id = t.id where t.id = 2;
+---------------------------+-----------------+------------+
| operation                 | startTS         | duration   |
+---------------------------+-----------------+------------+
| session.getTxnFuture      | 12:02:26.316169 | 3.014µs    |
|   ├─session.Execute       | 12:02:26.316166 | 867.827µs  |
|   ├─session.ParseSQL      | 12:02:26.316213 | 54.937µs   |
|   ├─executor.Compile      | 12:02:26.316297 | 379.766µs  |
|   ├─session.runStmt       | 12:02:26.316714 | 272.396µs  |
|   ├─session.CommitTxn     | 12:02:26.316955 | 9.873µs    |
|   ├─recordSet.Next        | 12:02:26.317053 | 449.804µs  |
|   ├─tableReader.Next      | 12:02:26.317115 | 324.444µs  |
|   └─tableReader.Next      | 12:02:26.317125 | 288.022µs  |
+---------------------------+-----------------+------------+
9 rows in set (0.01 sec)

from this two table with same name we don't know which table have question, also miss join execution info, and maybe trace insert is useful too.

@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

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

@jackysp jackysp added type/enhancement The issue or PR belongs to an enhancement. status/LGT2 Indicates that a PR has LGTM 2. labels Jan 13, 2019
@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

/run-common-test tidb-test=pr/715
/run-integration-common-test tidb-test=pr/715

@zhouqiang-cl
Copy link
Contributor

/run-common-test

@lysu
Copy link
Contributor

lysu commented Jan 13, 2019

/run-common-test tidb-test=pr/716
/run-integration-common-test tidb-test=pr/716

@zz-jason zz-jason merged commit 8ac79f3 into pingcap:master Jan 13, 2019
@tiancaiamao tiancaiamao deleted the trace-row branch March 24, 2020 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants