*: support set relation job #6206
Conversation
|
/run-all-tests |
|
Have you considered |
ddl/ddl_worker.go
Outdated
| if curJob.ID < job.ID { | ||
| continue | ||
| } | ||
| isRelated, err := curJob.IsRelatedJob(job) |
There was a problem hiding this comment.
if isRelated, err := curJob.IsRelatedJob(job); err == nil; {
if isRelated {
curJob.RelatedID = job.ID
break
}
} else {
return err
}
above way is more concise?
There was a problem hiding this comment.
The method you say can have a simpler way, as follows
if isRelated, err := curJob.IsRelatedJob(job); err == nil && isRelated; {
curJob.RelatedID = job.ID
break
} else if err != nil {
return errors.Trace(err)
}
But, I think my method is more readable in Go's way.
ddl/ddl_worker.go
Outdated
| @@ -91,6 +91,29 @@ func (d *ddl) isOwner() bool { | |||
| return isOwner | |||
| } | |||
|
|
|||
| // setJobRelation sets the current job's relation-ID to the ID of a job that is related to current job. | |||
| // The related job's ID must less than the current job's ID, and we need the largest one in the list. | |||
| func setJobRelation(t *meta.Meta, curJob *model.Job) error { | |||
There was a problem hiding this comment.
setJobRelation --> buildJobRelation is better?
There was a problem hiding this comment.
Does setJobRelation run in a transaction?
There was a problem hiding this comment.
I used t, so it's in a transaction.
model/ddl.go
Outdated
| return true, nil | ||
| } | ||
| if job.Type == ActionRenameTable { | ||
| var schemaID int64 |
There was a problem hiding this comment.
Name it oldSchemaID is more clear?
model/ddl.go
Outdated
| } | ||
|
|
||
| // IsRelatedJob returns whether job and job1 are related or not. | ||
| func (job *Job) IsRelatedJob(job1 *Job) (bool, error) { |
model/ddl.go
Outdated
| @@ -213,6 +215,41 @@ func (job *Job) String() string { | |||
| job.ID, job.Type, job.State, job.SchemaState, job.SchemaID, job.TableID, rowCount, len(job.Args), tsConvert2Time(job.StartTS), job.Error, job.ErrorCount, job.SnapshotVer) | |||
| } | |||
|
|
|||
| func (job *Job) isRelatedSchema(job1 *Job) (bool, error) { | |||
|
|
||
| length := int(meta.RIndex - meta.LIndex) | ||
| elements := make([][]byte, 0, length) | ||
| for index := meta.RIndex - 1; index >= meta.LIndex; index-- { |
There was a problem hiding this comment.
Can you be sure it's ordered.
There was a problem hiding this comment.
We use RPush to enqueue, so we know this order.
meta/meta_test.go
Outdated
| @@ -1,3 +1,4 @@ | |||
| // LGetAll gets all elements of this list in order from left to right. | |||
ddl/ddl_worker.go
Outdated
| @@ -91,6 +91,29 @@ func (d *ddl) isOwner() bool { | |||
| return isOwner | |||
| } | |||
|
|
|||
| // buildJobDependence sets the current job's dependence-ID that the current job depends on. | |||
| // The dependent job's ID must less than the current job's ID, and we need the largest one in the list. | |||
| func buildJobDependence(t *meta.Meta, curJob *model.Job) error { | |||
There was a problem hiding this comment.
Add comments for what is a dependence job?
model/ddl.go
Outdated
| @@ -127,6 +127,8 @@ type Job struct { | |||
| // StartTS uses timestamp allocated by TSO. | |||
| // Now it's the TS when we put the job to TiKV queue. | |||
| StartTS uint64 `json:"start_ts"` | |||
| // DependentID is the job's ID that the current job depends on. | |||
| DependentID int64 `json:"related_id"` | |||
There was a problem hiding this comment.
s/DependentID/DependencyID
s/related_id/dependency_id
| // buildJobDependence sets the curjob's dependency-ID. | ||
| // The dependency-job's ID must less than the current job's ID, and we need the largest one in the list. | ||
| func buildJobDependence(t *meta.Meta, curJob *model.Job) error { | ||
| jobs, err := t.GetAllDDLJobs() |
There was a problem hiding this comment.
Should we run this check in a transaction? For example, two DDLs come in the same time. Will this function detect the dependence?
There was a problem hiding this comment.
It's in a transaction. The t has a field of transaction.
|
/run-all-tests tidb-test=pr/485 |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
|
LGTM |
We will initially support the parallel DDL. We need to wait for PR(#6161) to merge before we can begin.
In order to prevent conflicts, I first implement the function of
setJobRelation.setJobRelation sets the current job's relation-ID to the ID of a job that is related to the current job.