ddl: set correct startHandle when add indices meets some errors #6897
Conversation
ddl/index.go
Outdated
|
|
||
| batchTasks = append(batchTasks, task) | ||
| if len(batchTasks) >= len(workers) || i == (len(kvRanges)-1) { | ||
| // Wait tasks finish. | ||
| err = w.handleReorgTasks(startTime, startHandle, reorgInfo, &totalAddedCount, workers, batchTasks) | ||
| err = w.handleReorgTasks(startTime, firstHandle, reorgInfo, &totalAddedCount, workers, batchTasks) |
There was a problem hiding this comment.
I guess the firstHandle argument is redundant.
You can always get it from batchTasks[0].start, right?
ddl/index.go
Outdated
| @@ -687,6 +689,14 @@ func (w *addIndexWorker) run(d *ddlCtx) { | |||
| } | |||
|
|
|||
| log.Debug("[ddl-reorg] got backfill index task:#v", task) | |||
|
|
|||
| if task.mockTask { | |||
There was a problem hiding this comment.
Why not gofail here directly?
There was a problem hiding this comment.
Here is in a multi-thread environment, so it's non-trivial to control the gofail run only once.
There was a problem hiding this comment.
Could we use addIndexWorker's offset to control this error run only once? Then we can use gofail here, and remove the variables in handleReorgTasks.
There was a problem hiding this comment.
@winkyao
I think it worth it to gofail here directly.
We can check the worker id to avoid race.
|
@tiancaiamao PTAL |
|
LGTM |
ciscoxll
added
status/LGT2
|
/run-all-tests |
ddl/index.go
Outdated
| for i, task := range batchTasks { | ||
| // gofail: var mockAddIndexErr bool | ||
| // if mockAddIndexErr && !gofailMockAddindexErrOnceGuard { |
There was a problem hiding this comment.
gofailMockAddindexErrOnceGuard is too long.
There was a problem hiding this comment.
To avoid name conflicts.
|
LGTM |
coocood
added
status/LGT3
|
LGTM |
Thank you for working on TiDB! Please read TiDB's CONTRIBUTING document BEFORE filing this PR.
What have you changed? (mandatory)
Before this PR, we will fail the added gofail test.
When we encounter an error, we will update incorrect handle to the DDL job. Then we will never backfill the skipped indices range, cause
admin check tablefailed.What are the type of the changes (mandatory)?
How has this PR been tested (mandatory)?
Unit test.
@zimulala @crazycs520 @ciscoxll PTAL