domain: Clean up the goroutine after closing a domain #5163
Conversation
|
/run-all-tests |
domain/domain.go
Outdated
| @@ -317,6 +317,7 @@ 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. | |||
|
PTAL @siddontang |
domain/domain.go
Outdated
| if err == nil { | ||
| return nil | ||
| } | ||
| // If the domain exits, we return this function. |
domain/domain.go
Outdated
| if err == nil { | ||
| return nil | ||
| } | ||
| // If the domain exits, we return this function. |
There was a problem hiding this comment.
s/this function/error immediately/
|
PTAL @tiancaiamao @coocood |
| @@ -474,6 +496,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) | |||
There was a problem hiding this comment.
why need to wait for this rountine exit?
There was a problem hiding this comment.
Normally the structure stops, then the gorountine will exit.
|
@zimulala |
|
SchemaSyncer belongs to ddl object, how about move it's logic to other place, out of When |
|
@coocood |
|
@tiancaiamao |
|
|
||
| for { | ||
| childCtx, cancel := goctx.WithTimeout(ctx, timeout) | ||
| err := syncer.Restart(childCtx) |
domain/domain.go
Outdated
| @@ -352,6 +352,28 @@ func (do *Domain) loadSchemaInLoop(lease time.Duration) { | |||
| } | |||
| } | |||
|
|
|||
| func (do *Domain) mustRestartSyncer() error { | |||
There was a problem hiding this comment.
Please add comments for this method.
|
PTAL @coocood @winkyao @tiancaiamao |
|
LGTM |
If we close the domain, maybe the channel of
syncer.Doneanddo.exitare both notified.If it selects the channel of
syncer.Doneand the NewSession function encounters an error likegrpc: the client connection is closing(the error isn’t thecontext doneerror), then the goroutine can't quit. So this domain will leave some dirty data after quitting.