*: support 'admin show ddl jobs <number>' grammar #7028

crazycs520 · 2018-07-10T12:23:21Z

What have you changed? (mandatory)

issue#7025
add "admin show ddl jobs [number]" grammar support.
this is use to support parallel ddl test in schrodinger-test
eg:

mysql root@127.0.0.1:(none)> admin show ddl jobs
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
| JOB_ID | DB_NAME | TABLE_NAME                           | JOB_TYPE     | SCHEMA_STATE | SCHEMA_ID | TABLE_ID | ROW_COUNT | START_TIME                        | STATE  |
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
| 545    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | add index    | public       | 1         | 489      | 0         | 2018-07-10 20:07:04.144 +0800 CST | synced |
| 544    | test    | a9ce39c0-f856-4ac0-9c80-78e32ac44481 | create table | public       | 1         | 543      | 0         | 2018-07-10 20:07:04.067 +0800 CST | synced |
| 542    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | drop column  | none         | 1         | 517      | 0         | 2018-07-10 20:07:03.366 +0800 CST | synced |
| 541    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:03.195 +0800 CST | synced |
| 540    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | drop index   | none         | 1         | 517      | 0         | 2018-07-10 20:07:03.139 +0800 CST | synced |
| 539    | test    | afe54691-648f-4574-a05a-f706b368e07b | add index    | public       | 1         | 496      | 1         | 2018-07-10 20:07:02.621 +0800 CST | synced |
| 538    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:02.506 +0800 CST | synced |
| 537    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:02.032 +0800 CST | synced |
| 536    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add index    | public       | 1         | 474      | 0         | 2018-07-10 20:07:01.86 +0800 CST  | synced |
| 535    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | drop index   | none         | 1         | 489      | 0         | 2018-07-10 20:07:01.694 +0800 CST | synced |
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
10 rows in set
Time: 0.079s
mysql root@127.0.0.1:(none)> admin show ddl jobs 20
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
| JOB_ID | DB_NAME | TABLE_NAME                           | JOB_TYPE     | SCHEMA_STATE | SCHEMA_ID | TABLE_ID | ROW_COUNT | START_TIME                        | STATE  |
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
| 545    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | add index    | public       | 1         | 489      | 0         | 2018-07-10 20:07:04.144 +0800 CST | synced |
| 544    | test    | a9ce39c0-f856-4ac0-9c80-78e32ac44481 | create table | public       | 1         | 543      | 0         | 2018-07-10 20:07:04.067 +0800 CST | synced |
| 542    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | drop column  | none         | 1         | 517      | 0         | 2018-07-10 20:07:03.366 +0800 CST | synced |
| 541    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:03.195 +0800 CST | synced |
| 540    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | drop index   | none         | 1         | 517      | 0         | 2018-07-10 20:07:03.139 +0800 CST | synced |
| 539    | test    | afe54691-648f-4574-a05a-f706b368e07b | add index    | public       | 1         | 496      | 1         | 2018-07-10 20:07:02.621 +0800 CST | synced |
| 538    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:02.506 +0800 CST | synced |
| 537    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add column   | public       | 1         | 474      | 0         | 2018-07-10 20:07:02.032 +0800 CST | synced |
| 536    | test    | f94383a9-bd1c-4a1e-bd1f-ec44af3e3c1e | add index    | public       | 1         | 474      | 0         | 2018-07-10 20:07:01.86 +0800 CST  | synced |
| 535    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | drop index   | none         | 1         | 489      | 0         | 2018-07-10 20:07:01.694 +0800 CST | synced |
| 534    | test    |                                      | drop table   | none         | 1         | 502      | 0         | 2018-07-10 20:07:01.673 +0800 CST | synced |
| 533    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | add index    | public       | 1         | 489      | 3         | 2018-07-10 20:07:01.312 +0800 CST | synced |
| 532    | test    | afe54691-648f-4574-a05a-f706b368e07b | add column   | public       | 1         | 496      | 0         | 2018-07-10 20:07:01.311 +0800 CST | synced |
| 531    | test    | d5ea4e33-4fe2-46ae-accb-82610ea1e0b1 | drop column  | none         | 1         | 489      | 0         | 2018-07-10 20:07:00.873 +0800 CST | synced |
| 530    | test    | afe54691-648f-4574-a05a-f706b368e07b | add column   | public       | 1         | 496      | 0         | 2018-07-10 20:07:00.872 +0800 CST | synced |
| 529    | test    |                                      | add index    | public       | 1         | 502      | 1         | 2018-07-10 20:07:00.419 +0800 CST | synced |
| 528    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | add column   | public       | 1         | 517      | 0         | 2018-07-10 20:07:00.417 +0800 CST | synced |
| 527    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | add column   | public       | 1         | 517      | 0         | 2018-07-10 20:06:59.972 +0800 CST | synced |
| 526    | test    | 0284e4b1-56e9-43c1-bcf5-0a82126fbaee | add index    | public       | 1         | 517      | 0         | 2018-07-10 20:06:59.97 +0800 CST  | synced |
| 525    | test    |                                      | add column   | public       | 1         | 502      | 0         | 2018-07-10 20:06:59.719 +0800 CST | synced |
+--------+---------+--------------------------------------+--------------+--------------+-----------+----------+-----------+-----------------------------------+--------+
20 rows in set
Time: 0.064s
mysql root@127.0.0.1:(none)>

What are the type of the changes (mandatory)?

New feature (non-breaking change which adds functionality)

How has this PR been tested (mandatory)?

unit test

zimulala · 2018-07-11T01:54:45Z

executor/executor.go

@@ -247,7 +248,7 @@ func (e *ShowDDLJobQueriesExec) Open(ctx context.Context) error {
 		return errors.Trace(err)
 	}
 	// TODO: need to return the job that the user needs.
-	historyJobs, err := admin.GetHistoryDDLJobs(e.ctx.Txn())
+	historyJobs, err := admin.GetHistoryDDLJobs(e.ctx.Txn(), 10)


Could we define 10 as a constant?

zimulala · 2018-07-11T01:56:31Z

executor/executor_test.go

+	err = r.Next(ctx, chk)
+	c.Assert(err, IsNil)
+	row = chk.GetRow(0)
+	c.Assert(row.Len(), Equals, 10)


Why is its value 10 is not 20?

I think it the first-row length.

Do we need to check the number of jobs in the result?

The number of jobs in the result cannot be ensured, it is related to the actual DDL jobs.

XuHuaiyu · 2018-07-11T14:38:08Z

util/admin/admin.go

@@ -137,18 +137,21 @@ func GetDDLJobs(txn kv.Transaction) ([]*model.Job, error) {
 // MaxHistoryJobs is exported for testing.
 const MaxHistoryJobs = 10

+// DefHistoryJobs is default value of the default number of history job
+const DefHistoryJobs = 10


s/DefHistoryJobs/DefHistoryJobsNum?

XuHuaiyu · 2018-07-11T14:38:45Z

executor/executor.go

-
-	historyJobs, err := admin.GetHistoryDDLJobs(e.ctx.Txn())
+	if e.jobNumber == 0 {
+		e.jobNumber = 10


s/ 10/ admin/DefHistoryJobs?

XuHuaiyu · 2018-07-11T14:41:12Z

parser/parser_test.go

@@ -413,6 +413,7 @@ func (s *testParserSuite) TestDMLStmt(c *C) {
 		// for admin
 		{"admin show ddl;", true},
 		{"admin show ddl jobs;", true},
+		{"admin show ddl jobs 20;", true},


add test for
admin show ddl jobs -1;

zz-jason · 2018-07-12T03:56:47Z

executor/executor.go

@@ -247,7 +248,7 @@ func (e *ShowDDLJobQueriesExec) Open(ctx context.Context) error {
 		return errors.Trace(err)
 	}
 	// TODO: need to return the job that the user needs.


this todo can be removed?

zz-jason · 2018-07-12T03:58:23Z

util/admin/admin.go

@@ -137,18 +137,21 @@ func GetDDLJobs(txn kv.Transaction) ([]*model.Job, error) {
 // MaxHistoryJobs is exported for testing.
 const MaxHistoryJobs = 10

+// DefHistoryJobsNum is default value of the default number of history job
+const DefHistoryJobsNum = 10


how about s/DefHistoryJobsNum/DefNumHistoryJobs/ ?

zz-jason · 2018-07-12T03:59:39Z

util/admin/admin.go

-// The maximum count of history jobs is MaxHistoryJobs.
-func GetHistoryDDLJobs(txn kv.Transaction) ([]*model.Job, error) {
+// The maximum count of history jobs is num.
+func GetHistoryDDLJobs(txn kv.Transaction, num int) ([]*model.Job, error) {


how about s/num/maxNumJobs/ ?

zz-jason · 2018-07-12T06:11:42Z

LGTM

zz-jason · 2018-07-12T06:12:53Z

@zimulala @jackysp @XuHuaiyu PTAL

XuHuaiyu

LGTM

zz-jason · 2018-07-12T06:26:37Z

/run-all-tests

crazycs520 added 2 commits July 10, 2018 20:14

*: support 'admin show ddl jobs <number>' grammar

c5f60ac

add comment

3e84e5c

zimulala reviewed Jul 11, 2018

View reviewed changes

add DefHistoryJobs constant

5e88f8d

XuHuaiyu reviewed Jul 11, 2018

View reviewed changes

crazycs520 added 2 commits July 12, 2018 11:02

add test

3751837

rename DefHistoryJobs to DefHistoryJobsNum

c0802f9

zz-jason reviewed Jul 12, 2018

View reviewed changes

rename refactor and remove TODO

baf542e

zz-jason added type/enhancement The issue or PR belongs to an enhancement. status/LGT1 Indicates that a PR has LGTM 1. labels Jul 12, 2018

Merge branch 'master' into admn-show-ddl-job-num

1eb48bb

XuHuaiyu approved these changes Jul 12, 2018

View reviewed changes

zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 12, 2018

zz-jason merged commit 4616636 into pingcap:master Jul 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support 'admin show ddl jobs <number>' grammar #7028

*: support 'admin show ddl jobs <number>' grammar #7028

crazycs520 commented Jul 10, 2018

zimulala Jul 11, 2018

crazycs520 Jul 11, 2018

zimulala Jul 11, 2018

jackysp Jul 11, 2018

crazycs520 Jul 11, 2018

zimulala Jul 11, 2018

crazycs520 Jul 12, 2018

XuHuaiyu Jul 11, 2018

crazycs520 Jul 12, 2018

XuHuaiyu Jul 11, 2018

XuHuaiyu Jul 11, 2018

crazycs520 Jul 12, 2018

zz-jason Jul 12, 2018

zz-jason Jul 12, 2018

zz-jason Jul 12, 2018

crazycs520 Jul 12, 2018

zz-jason commented Jul 12, 2018

zz-jason commented Jul 12, 2018

XuHuaiyu left a comment

zz-jason commented Jul 12, 2018

*: support 'admin show ddl jobs <number>' grammar #7028

*: support 'admin show ddl jobs <number>' grammar #7028

Conversation

crazycs520 commented Jul 10, 2018

What have you changed? (mandatory)

What are the type of the changes (mandatory)?

How has this PR been tested (mandatory)?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason commented Jul 12, 2018

zz-jason commented Jul 12, 2018

XuHuaiyu left a comment

Choose a reason for hiding this comment

zz-jason commented Jul 12, 2018