search

pingcap / dm

Data Migration Platform
Apache License 2.0
456 stars 188 forks source link

shardddl: fix data race in test #2236

Closed sleepymole closed 3 years ago

sleepymole commented 3 years ago

What problem does this PR solve?

There is data race in test TestOperationEtcd:

WARNING: DATA RACE
Write at 0x00c00090bb70 by goroutine 65:
  runtime.closechan()
      /usr/lib/go/src/runtime/chan.go:355 +0x0
  github.com/pingcap/dm/pkg/shardddl/pessimism.(*testForEtcd).TestOperationEtcd()
      /home/xyj/Projects/dm/pkg/shardddl/pessimism/operation_test.go:77 +0x1667
  runtime.call16()
      /usr/lib/go/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /usr/lib/go/src/reflect/value.go:339 +0x149
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /home/xyj/.local/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850 +0xa7c
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /home/xyj/.local/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739 +0x244

Previous read at 0x00c00090bb70 by goroutine 68:
  runtime.chansend()
      /usr/lib/go/src/runtime/chan.go:158 +0x0
  github.com/pingcap/dm/pkg/shardddl/pessimism.watchOperation()
      /home/xyj/Projects/dm/pkg/shardddl/pessimism/operation.go:271 +0x1684
  github.com/pingcap/dm/pkg/shardddl/pessimism.WatchOperationPut()
      /home/xyj/Projects/dm/pkg/shardddl/pessimism/operation.go:208 +0x1a6
  github.com/pingcap/dm/pkg/shardddl/pessimism.(*testForEtcd).TestOperationEtcd·dwrap·19()
      /home/xyj/Projects/dm/pkg/shardddl/pessimism/operation_test.go:71 +0x195

What is changed and how it works?

Use watchExactPutOperations to watch exact number of operations.

Check List

Tests

Code changes

Side effects

Related changes

ti-chi-bot commented 3 years ago

[REVIEW NOTIFICATION]

This pull request has been approved by:

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment. After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review. Reviewer can cancel approval by submitting a request changes review.
sleepymole commented 3 years ago

Data race also occurred in test of shardddl/optimism:

WARNING: DATA RACE
Write at 0x00c0000c1990 by goroutine 7:
  runtime.closechan()
      /usr/lib/go/src/runtime/chan.go:355 +0x0
  github.com/pingcap/dm/dm/master/shardddl.(*testOptimist).testOptimist()
      /home/xyj/Projects/dm/dm/master/shardddl/optimist_test.go:381 +0x9924
  github.com/pingcap/dm/dm/master/shardddl.(*testOptimist).TestOptimist()
      /home/xyj/Projects/dm/dm/master/shardddl/optimist_test.go:155 +0x339
  runtime.call16()
      /usr/lib/go/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /usr/lib/go/src/reflect/value.go:339 +0x149
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /home/xyj/.local/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850 +0xa7c
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /home/xyj/.local/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739 +0x244

Previous read at 0x00c0000c1990 by goroutine 91:
  runtime.chansend()
      /usr/lib/go/src/runtime/chan.go:158 +0x0
  github.com/pingcap/dm/pkg/shardddl/optimism.WatchOperationPut()
      /home/xyj/Projects/dm/pkg/shardddl/optimism/operation.go:271 +0x10a4
  github.com/pingcap/dm/dm/master/shardddl.(*testOptimist).testOptimist·dwrap·23()
      /home/xyj/Projects/dm/dm/master/shardddl/optimist_test.go:376 +0x26a
sleepymole commented 3 years ago

All related tests are fixed now.

lance6716 commented 3 years ago

/merge

ti-chi-bot commented 3 years ago

This pull request has been accepted and is ready to merge.

Commit hash: 16621864314755b3f42e330a4abb9c29d1a31bec

ti-chi-bot commented 3 years ago

In response to a cherrypick label: new pull request created: #2240.