Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leak test pdutil.TestScheduler #28816

Closed
Tracked by #25899
djshow832 opened this issue Oct 14, 2021 · 3 comments · Fixed by #28932
Closed
Tracked by #25899

Leak test pdutil.TestScheduler #28816

djshow832 opened this issue Oct 14, 2021 · 3 comments · Fixed by #28932
Assignees
Labels
component/br This issue is related to BR of TiDB. component/test severity/major type/bug The issue is confirmed as a bug.

Comments

@djshow832
Copy link
Contributor

Bug Report

[2021-10-14T07:19:31.756Z] goleak: Errors on successful test run: found unexpected goroutines:
[2021-10-14T07:19:31.756Z] [Goroutine 265 in state chan receive, with github.com/pingcap/tidb/br/pkg/pdutil.TestScheduler.func5 on top of the stack:
[2021-10-14T07:19:31.756Z] goroutine 265 [chan receive]:
[2021-10-14T07:19:31.756Z] github.com/pingcap/tidb/br/pkg/pdutil.TestScheduler.func5(0xc00060e2a0)
[2021-10-14T07:19:31.756Z] 	/home/jenkins/agent/workspace/tidb_ghpr_check_2/go/src/github.com/pingcap/tidb/br/pkg/pdutil/pd_serial_test.go:74 +0x34
[2021-10-14T07:19:31.756Z] created by github.com/pingcap/tidb/br/pkg/pdutil.TestScheduler
[2021-10-14T07:19:31.756Z] 	/home/jenkins/agent/workspace/tidb_ghpr_check_2/go/src/github.com/pingcap/tidb/br/pkg/pdutil/pd_serial_test.go:73 +0x825
[2021-10-14T07:19:31.756Z] ]
[2021-10-14T07:19:31.756Z] FAIL	github.com/pingcap/tidb/br/pkg/pdutil	0.674s

See https://ci.pingcap.net/blue/organizations/jenkins/tidb_ghpr_check_2/detail/tidb_ghpr_check_2/38433/pipeline

@djshow832 djshow832 added the type/bug The issue is confirmed as a bug. label Oct 14, 2021
@aytrack aytrack added component/br This issue is related to BR of TiDB. component/test severity/major labels Oct 14, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Oct 18, 2021

@unconsolable please take a look at this test. Possibly we should handle schedulerPauseCh gracefully.

@unconsolable
Copy link
Contributor

_, err = pdController.pauseSchedulersAndConfigWith(ctx, []string{scheduler}, cfg, mock)
require.NoError(t, err)
go func() {
<-schedulerPauseCh
}()
err = pdController.resumeSchedulerWith(ctx, []string{scheduler}, mock)

IMHO this goroutine creation should be removed, as pauseSchedulersAndConfigWith success, it will create a goroutine and select on it.

tidb/br/pkg/pdutil/pd.go

Lines 448 to 450 in ae346a5

case <-p.schedulerPauseCh:
log.Info("exit pause scheduler and configs successful")
return

Also, in (*PdController).Close(), it will close the chan and the pdClient. as in this scenario pdClient is nill, we can close the channel in defer.

@github-actions
Copy link

Please check whether the issue should be labeled with 'affects-x.y' or 'fixes-x.y.z', and then remove 'needs-more-info' label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/br This issue is related to BR of TiDB. component/test severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants