Skip to content

Commit

Permalink
domain: Clean up the goroutine after closing a domain (pingcap#5163)
Browse files Browse the repository at this point in the history
  • Loading branch information
zimulala authored and coocood committed Nov 22, 2017
1 parent 4d09087 commit 9c36002
Showing 1 changed file with 28 additions and 2 deletions.
30 changes: 28 additions & 2 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ func (do *Domain) Reload() error {
}

func (do *Domain) loadSchemaInLoop(lease time.Duration) {
defer do.wg.Done()
// Lease renewal can run at any frequency.
// Use lease/2 here as recommend by paper.
// TODO: Reset ticker or make interval longer.
ticker := time.NewTicker(lease / 2)
defer ticker.Stop()
syncer := do.ddl.SchemaSyncer()
Expand All @@ -340,7 +340,7 @@ func (do *Domain) loadSchemaInLoop(lease time.Duration) {
// The schema syncer stops, we need stop the schema validator to synchronize the schema version.
log.Info("[ddl] reload schema in loop, schema syncer need restart")
do.SchemaValidator.Stop()
err := syncer.Restart(goctx.Background())
err := do.mustRestartSyncer()
if err != nil {
log.Errorf("[ddl] reload schema in loop, schema syncer restart err %v", errors.ErrorStack(err))
break
Expand All @@ -352,6 +352,31 @@ func (do *Domain) loadSchemaInLoop(lease time.Duration) {
}
}

// mustRestartSyncer tries to restart the SchemaSyncer.
// It returns until it's successful or the domain is stoped.
func (do *Domain) mustRestartSyncer() error {
ctx := goctx.Background()
syncer := do.ddl.SchemaSyncer()
timeout := 5 * time.Second

for {
childCtx, cancel := goctx.WithTimeout(ctx, timeout)
err := syncer.Restart(childCtx)
cancel()
if err == nil {
return nil
}
// If the domain has stopped, we return an error immediately.
select {
case <-do.exit:
return errors.Trace(err)
default:
}
time.Sleep(time.Second)
log.Infof("[ddl] restart the schema syncer failed %v", err)
}
}

// Close closes the Domain and release its resource.
func (do *Domain) Close() {
terror.Log(errors.Trace(do.ddl.Stop()))
Expand Down Expand Up @@ -474,6 +499,7 @@ func NewDomain(store kv.Storage, ddlLease time.Duration, statsLease time.Duratio
// Only when the store is local that the lease value is 0.
// If the store is local, it doesn't need loadSchemaInLoop.
if ddlLease > 0 {
d.wg.Add(1)
// Local store needs to get the change information for every DDL state in each session.
go d.loadSchemaInLoop(ddlLease)
}
Expand Down

0 comments on commit 9c36002

Please sign in to comment.