Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

fix data race in import unit(loader) (#349) #353

Merged
merged 6 commits into from
Nov 15, 2019
Merged

fix data race in import unit(loader) (#349) #353

merged 6 commits into from
Nov 15, 2019

Conversation

sre-bot
Copy link

@sre-bot sre-bot commented Nov 14, 2019

cherry-pick #349 to release-1.0

What problem does this PR solve?

find a data race in import unit(loader)

panic: send on closed channel

goroutine 44191 [running]:
github.com/pingcap/dm/loader.(*Worker).run.func1()
	/home/jenkins/agent/workspace/build_dm_master/go/src/github.com/pingcap/dm/loader/loader.go:155 +0x47b
created by github.com/pingcap/dm/loader.(*Worker).run
	/home/jenkins/agent/workspace/build_dm_master/go/src/github.com/pingcap/dm/loader/loader.go:175 +0x2ba
panic: send on closed channel

What is changed and how it works?

  • make sure the worker clean up all its goroutine when it exits
  • It's very weird, dump unit start a doJob groutine for each dump files, not for each worker

IANTHEREAL and others added 6 commits November 14, 2019 12:49
@IANTHEREAL @sre-bot
fix data race in import unit(loader)
ceac2bb
@IANTHEREAL @sre-bot
add import goroutine leak integration test
ebbd060
@IANTHEREAL @sre-bot
fix goroutine leak in restoreData
060fb92
@IANTHEREAL @sre-bot
address comment
36ba61a
@IANTHEREAL @sre-bot
refine old logic of restoreData
3ee0c41
@csuzhangxc @sre-bot
worker: fix data race
a6c0c90 
```
2019-11-13T14:04:06.914Z] WARNING: DATA RACE

[2019-11-13T14:04:06.914Z] Write at 0x00c0003bb5a0 by goroutine 97:

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*SubTask).Init()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/subtask.go:111 +0xf5

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*SubTask).Run()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/subtask.go:169 +0x2cc

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*SubTask).Resume()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/subtask.go:412 +0x80e

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*Worker).handleTask()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:880 +0x1ecc

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*Worker).Start.func1()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:178 +0x80

[2019-11-13T14:04:06.914Z]

[2019-11-13T14:04:06.914Z] Previous read at 0x00c0003bb5a0 by goroutine 147:

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*Worker).doFetchDDLInfo()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:398 +0x203

[2019-11-13T14:04:06.914Z]   github.com/pingcap/dm/dm/worker.(*Worker).FetchDDLInfo.func1()

[2019-11-13T14:04:06.914Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:374 +0x98

[2019-11-13T14:04:06.915Z]

[2019-11-13T14:04:06.915Z] Goroutine 97 (running) created at:

[2019-11-13T14:04:06.915Z]   github.com/pingcap/dm/dm/worker.(*Worker).Start()

[2019-11-13T14:04:06.915Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:176 +0x26f

[2019-11-13T14:04:06.915Z]   github.com/pingcap/dm/dm/worker.(*Server).Start.func1()

[2019-11-13T14:04:06.915Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/server.go:87 +0x96

[2019-11-13T14:04:06.915Z]

[2019-11-13T14:04:06.915Z] Goroutine 147 (running) created at:

[2019-11-13T14:04:06.915Z]   github.com/pingcap/dm/dm/worker.(*Worker).FetchDDLInfo()

[2019-11-13T14:04:06.915Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/worker.go:372 +0x1f4

[2019-11-13T14:04:06.915Z]   github.com/pingcap/dm/dm/worker.(*Server).FetchDDLInfo()

[2019-11-13T14:04:06.915Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/worker/server.go:286 +0x28b

[2019-11-13T14:04:06.915Z]   github.com/pingcap/dm/dm/pb._Worker_FetchDDLInfo_Handler()

[2019-11-13T14:04:06.915Z]       /home/jenkins/agent/workspace/dm_ghpr_test/go/src/github.com/pingcap/dm/dm/pb/dmworker.pb.go:3865 +0xe5

[2019-11-13T14:04:06.915Z]   google.golang.org/grpc.(*Server).processStreamingRPC()

[2019-11-13T14:04:06.915Z]       /go/pkg/mod/google.golang.org/[email protected]/server.go:1199 +0x15f2

[2019-11-13T14:04:06.915Z]   google.golang.org/grpc.(*Server).handleStream()

[2019-11-13T14:04:06.915Z]       /go/pkg/mod/google.golang.org/[email protected]/server.go:1279 +0x12e5

[2019-11-13T14:04:06.915Z]   google.golang.org/grpc.(*Server).serveStreams.func1.1()

[2019-11-13T14:04:06.915Z]       /go/pkg/mod/google.golang.org/[email protected]/server.go:710 +0xac
```
@sre-bot sre-bot mentioned this pull request Nov 14, 2019
Merged
@sre-bot sre-bot added priority/normal Minor change, requires approval from ≥1 primary reviewer needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated type/bug-fix Bug fix type/cherry-pick This PR is just a cherry-pick (backport) labels Nov 14, 2019
@csuzhangxc
Copy link
Member

/run-all-tests tidb=release-3.0

csuzhangxc
csuzhangxc reviewed Nov 14, 2019
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

❗ No coverage uploaded for pull request base (release-1.0@9eb6ac9). Click here to learn what that means.
The diff coverage is 8.1967%.

@@               Coverage Diff                @@
##             release-1.0       #353   +/-   ##
================================================
  Coverage               ?   57.4761%           
================================================
  Files                  ?        158           
  Lines                  ?      16031           
  Branches               ?          0           
================================================
  Hits                   ?       9214           
  Misses                 ?       5914           
  Partials               ?        903

@csuzhangxc csuzhangxc added the status/LGT1 One reviewer already commented LGTM label Nov 15, 2019
@csuzhangxc
Copy link
Member

@WangXiangUSTC PTAL

IANTHEREAL
IANTHEREAL approved these changes Nov 15, 2019
View reviewed changes
Copy link
Collaborator

@IANTHEREAL IANTHEREAL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IANTHEREAL IANTHEREAL merged commit fb6cd98 into pingcap:release-1.0 Nov 15, 2019
@IANTHEREAL IANTHEREAL added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Nov 15, 2019
@ericsyh ericsyh added this to the v1.0.2 milestone Nov 22, 2019
ericsyh added a commit that referenced this pull request Dec 13, 2019
@ericsyh
Changelog: bump to v1.0.3 (#425)
f7fb8bb 
* bump to v1.0.3

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* address comments

* update the release notes

* update #353
sre-bot pushed a commit to sre-bot/dm that referenced this pull request Dec 13, 2019
@ericsyh @sre-bot
update pingcap#353
c610fbe
glorv pushed a commit to glorv/dm that referenced this pull request Dec 25, 2019
@ericsyh @glorv
Changelog: bump to v1.0.3 (pingcap#425)
be3e1d1 
* bump to v1.0.3

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* address comments

* update the release notes

* update pingcap#353
@csuzhangxc csuzhangxc added already-update-release-note The release note is updated. Add this label once the release note is updated and removed needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Mar 13, 2020
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
@ericsyh
Changelog: bump to v1.0.3 (pingcap#425)
1ecb5ea 
* bump to v1.0.3

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* Update CHANGELOG.md

Co-Authored-By: Xuecheng Zhang <[email protected]>

* address comments

* update the release notes

* update pingcap#353
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@csuzhangxc csuzhangxc csuzhangxc left review comments

@IANTHEREAL IANTHEREAL IANTHEREAL approved these changes

@WangXiangUSTC WangXiangUSTC Awaiting requested review from WangXiangUSTC

Assignees
No one assigned
Labels
already-update-release-note The release note is updated. Add this label once the release note is updated priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/bug-fix Bug fix type/cherry-pick This PR is just a cherry-pick (backport)
Projects
None yet
Milestone
v1.0.3
Development

Successfully merging this pull request may close these issues.
4 participants
@sre-bot @csuzhangxc @IANTHEREAL @ericsyh