-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
*: support 'admin show ddl jobs <number>' grammar #7028
Changes from 3 commits
c5f60ac
3e84e5c
5e88f8d
3751837
c0802f9
baf542e
1eb48bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,9 +221,10 @@ func (e *ShowDDLExec) Next(ctx context.Context, chk *chunk.Chunk) error { | |
type ShowDDLJobsExec struct { | ||
baseExecutor | ||
|
||
cursor int | ||
jobs []*model.Job | ||
is infoschema.InfoSchema | ||
cursor int | ||
jobs []*model.Job | ||
jobNumber int64 | ||
is infoschema.InfoSchema | ||
} | ||
|
||
// ShowDDLJobQueriesExec represents a show DDL job queries executor. | ||
|
@@ -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(), admin.DefHistoryJobs) | ||
if err != nil { | ||
return errors.Trace(err) | ||
} | ||
|
@@ -288,8 +289,10 @@ func (e *ShowDDLJobsExec) Open(ctx context.Context) error { | |
if err != nil { | ||
return errors.Trace(err) | ||
} | ||
|
||
historyJobs, err := admin.GetHistoryDDLJobs(e.ctx.Txn()) | ||
if e.jobNumber == 0 { | ||
e.jobNumber = 10 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/ 10/ admin/DefHistoryJobs? |
||
} | ||
historyJobs, err := admin.GetHistoryDDLJobs(e.ctx.Txn(), int(e.jobNumber)) | ||
if err != nil { | ||
return errors.Trace(err) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,13 +181,23 @@ func (s *testSuite) TestAdmin(c *C) { | |
c.Assert(row.Len(), Equals, 10) | ||
txn, err = s.store.Begin() | ||
c.Assert(err, IsNil) | ||
historyJobs, err := admin.GetHistoryDDLJobs(txn) | ||
historyJobs, err := admin.GetHistoryDDLJobs(txn, admin.DefHistoryJobs) | ||
c.Assert(len(historyJobs), Greater, 1) | ||
c.Assert(len(row.GetString(1)), Greater, 0) | ||
c.Assert(err, IsNil) | ||
c.Assert(row.GetInt64(0), Equals, historyJobs[0].ID) | ||
c.Assert(err, IsNil) | ||
|
||
r, err = tk.Exec("admin show ddl jobs 20") | ||
c.Assert(err, IsNil) | ||
chk = r.NewChunk() | ||
err = r.Next(ctx, chk) | ||
c.Assert(err, IsNil) | ||
row = chk.GetRow(0) | ||
c.Assert(row.Len(), Equals, 10) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is its value 10 is not 20? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it the first-row length. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ye~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to check the number of jobs in the result? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The number of jobs in the result cannot be ensured, it is related to the actual DDL jobs. |
||
c.Assert(row.GetInt64(0), Equals, historyJobs[0].ID) | ||
c.Assert(err, IsNil) | ||
|
||
// show DDL job queries test | ||
tk.MustExec("use test") | ||
tk.MustExec("drop table if exists admin_test2") | ||
|
@@ -196,7 +206,7 @@ func (s *testSuite) TestAdmin(c *C) { | |
result.Check(testkit.Rows()) | ||
result = tk.MustQuery(`admin show ddl job queries 1, 2, 3, 4`) | ||
result.Check(testkit.Rows()) | ||
historyJob, err := admin.GetHistoryDDLJobs(txn) | ||
historyJob, err := admin.GetHistoryDDLJobs(txn, admin.DefHistoryJobs) | ||
result = tk.MustQuery(fmt.Sprintf("admin show ddl job queries %d", historyJob[0].ID)) | ||
result.Check(testkit.Rows(historyJob[0].Query)) | ||
c.Assert(err, IsNil) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
{"admin show ddl job queries 1", true}, | ||
{"admin show ddl job queries 1, 2, 3, 4", true}, | ||
{"admin check table t1, t2;", true}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/DefHistoryJobs/DefHistoryJobsNum? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
// GetHistoryDDLJobs returns the DDL history jobs and an error. | ||
// 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about s/num/maxNumJobs/ ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
t := meta.NewMeta(txn) | ||
jobs, err := t.GetAllHistoryDDLJobs() | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
|
||
jobsLen := len(jobs) | ||
if jobsLen > MaxHistoryJobs { | ||
start := jobsLen - MaxHistoryJobs | ||
if jobsLen > num { | ||
start := jobsLen - num | ||
jobs = jobs[start:] | ||
} | ||
jobsLen = len(jobs) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this todo can be removed?