Skip to content

Commit

Permalink
ddl: handle context done after sending DDL jobs (#57945) (#57989)
Browse files Browse the repository at this point in the history
close #57863
  • Loading branch information
ti-chi-bot authored Dec 4, 2024
1 parent 1df27e0 commit 012bfdd
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
1 change: 1 addition & 0 deletions pkg/ddl/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,7 @@ func (d *ddl) close() {

startTime := time.Now()
d.cancel()
failpoint.InjectCall("afterDDLCloseCancel")
d.wg.Wait()
// when run with real-tikv, the lifecycle of ownerManager is managed by globalOwnerManager,
// when run with uni-store BreakCampaignLoop is same as Close.
Expand Down
13 changes: 10 additions & 3 deletions pkg/ddl/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6585,8 +6585,15 @@ func (e *executor) DoDDLJobWrapper(ctx sessionctx.Context, jobW *JobWrapper) (re
}
})

// worker should restart to continue handling tasks in limitJobCh, and send back through jobW.err
result := <-jobW.ResultCh[0]
var result jobSubmitResult
select {
case <-e.ctx.Done():
logutil.DDLLogger().Info("DoDDLJob will quit because context done")
return e.ctx.Err()
case res := <-jobW.ResultCh[0]:
// worker should restart to continue handling tasks in limitJobCh, and send back through jobW.err
result = res
}
// job.ID must be allocated after previous channel receive returns nil.
jobID, err := result.jobID, result.err
defer e.delJobDoneCh(jobID)
Expand Down Expand Up @@ -6656,7 +6663,7 @@ func (e *executor) DoDDLJobWrapper(ctx sessionctx.Context, jobW *JobWrapper) (re
ticker = updateTickerInterval(ticker, 10*e.lease, ddlAction, i)
case <-e.ctx.Done():
logutil.DDLLogger().Info("DoDDLJob will quit because context done")
return context.Canceled
return e.ctx.Err()
}

// If the connection being killed, we need to CANCEL the DDL job.
Expand Down
14 changes: 14 additions & 0 deletions pkg/ddl/job_submitter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,3 +581,17 @@ func TestGenGIDAndInsertJobsWithRetryOnErr(t *testing.T) {
require.True(t, ok)
require.Equal(t, newGID-1, jobs[0].TableID)
}

func TestSubmitJobAfterDDLIsClosed(t *testing.T) {
store, dom := testkit.CreateMockStoreAndDomain(t, mockstore.WithStoreType(mockstore.EmbedUnistore))
tk := testkit.NewTestKit(t, store)

var ddlErr error
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/afterDDLCloseCancel", func() {
ddlErr = tk.ExecToErr("create database test2;")
})
err := dom.DDL().Stop()
require.NoError(t, err)
require.Error(t, ddlErr)
require.Equal(t, "context canceled", ddlErr.Error())
}

0 comments on commit 012bfdd

Please sign in to comment.