test: fix ctx timeout in advance before test success in ut #2189

Ehco1996 · 2021-09-30T02:31:42Z

What problem does this PR solve?

part of #2073

What is changed and how it works?

as title

Check List

Tests

Unit test

Code changes

Side effects

Related changes

Need to cherry-pick to the release branch

ti-chi-bot · 2021-09-30T02:31:43Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lance6716
okJiang

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.

dm/master/election_test.go

Ehco1996 · 2021-09-30T03:42:14Z

fix this in a38581f

Ehco1996 · 2021-09-30T05:25:51Z

fix election_test.go:216: testElectionSuite.TestElection2After1 in 69cf5ca

Ehco1996 · 2021-09-30T05:57:09Z

data race in here

==================
WARNING: DATA RACE
Read at 0x00c000a820d8 by goroutine 35:
  internal/race.Read()
      /usr/local/go/src/internal/race/race.go:37 +0x44
  sync.(*WaitGroup).Add()
      /usr/local/go/src/sync/waitgroup.go:71 +0x1c4
  github.com/pingcap/dm/pkg/election.(*Election).campaignLoop()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:249 +0x73a
  github.com/pingcap/dm/pkg/election.NewElection.func1()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:143 +0x139

Previous write at 0x00c000a820d8 by goroutine 23:
  internal/race.Write()
      /usr/local/go/src/internal/race/race.go:41 +0x44
  sync.(*WaitGroup).Wait()
      /usr/local/go/src/sync/waitgroup.go:128 +0x192
  github.com/pingcap/dm/pkg/election.(*Election).campaignLoop.func4()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:254 +0x96
  github.com/pingcap/dm/pkg/election.(*Election).CancelEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:452 +0x195
  github.com/pingcap/dm/pkg/election.(*testElectionSuite).TestElectionEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:354 +0x2704
  runtime.call16()
      /usr/local/go/src/runtime/asm_amd64.s:550 +0x3d
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:337 +0x167
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:850 +0x799
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:739 +0x152

Goroutine 35 (running) created at:
  github.com/pingcap/dm/pkg/election.NewElection()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:141 +0xc48
  github.com/pingcap/dm/pkg/election.(*testElectionSuite).TestElectionEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:308 +0x676
  runtime.call16()
      /usr/local/go/src/runtime/asm_amd64.s:550 +0x3d
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:337 +0x167
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:850 +0x799
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:739 +0x152

Goroutine 23 (running) created at:
  github.com/pingcap/check.(*suiteRunner).forkCall()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:734 +0x764
  github.com/pingcap/check.(*suiteRunner).forkTest()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:832 +0x246
  github.com/pingcap/check.(*suiteRunner).doRun()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:666 +0x184
  github.com/pingcap/check.(*suiteRunner).run()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:696 +0x1ee
  github.com/pingcap/check.Run()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:150 +0xa4
  github.com/pingcap/check.RunAll()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:117 +0xad3
  github.com/pingcap/check.TestingT()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:99 +0xb08
  github.com/pingcap/dm/pkg/election.TestSuite()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:39 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1193 +0x2a4
==================

Ehco1996 · 2021-09-30T07:28:09Z

/verify

Ehco1996 · 2021-09-30T07:36:51Z

data race in here

==================
WARNING: DATA RACE
Read at 0x00c000a820d8 by goroutine 35:
  internal/race.Read()
      /usr/local/go/src/internal/race/race.go:37 +0x44
  sync.(*WaitGroup).Add()
      /usr/local/go/src/sync/waitgroup.go:71 +0x1c4
  github.com/pingcap/dm/pkg/election.(*Election).campaignLoop()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:249 +0x73a
  github.com/pingcap/dm/pkg/election.NewElection.func1()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:143 +0x139

Previous write at 0x00c000a820d8 by goroutine 23:
  internal/race.Write()
      /usr/local/go/src/internal/race/race.go:41 +0x44
  sync.(*WaitGroup).Wait()
      /usr/local/go/src/sync/waitgroup.go:128 +0x192
  github.com/pingcap/dm/pkg/election.(*Election).campaignLoop.func4()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:254 +0x96
  github.com/pingcap/dm/pkg/election.(*Election).CancelEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:452 +0x195
  github.com/pingcap/dm/pkg/election.(*testElectionSuite).TestElectionEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:354 +0x2704
  runtime.call16()
      /usr/local/go/src/runtime/asm_amd64.s:550 +0x3d
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:337 +0x167
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:850 +0x799
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:739 +0x152

Goroutine 35 (running) created at:
  github.com/pingcap/dm/pkg/election.NewElection()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election.go:141 +0xc48
  github.com/pingcap/dm/pkg/election.(*testElectionSuite).TestElectionEvictLeader()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:308 +0x676
  runtime.call16()
      /usr/local/go/src/runtime/asm_amd64.s:550 +0x3d
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:337 +0x167
  github.com/pingcap/check.(*suiteRunner).forkTest.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:850 +0x799
  github.com/pingcap/check.(*suiteRunner).forkCall.func1()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:739 +0x152

Goroutine 23 (running) created at:
  github.com/pingcap/check.(*suiteRunner).forkCall()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:734 +0x764
  github.com/pingcap/check.(*suiteRunner).forkTest()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:832 +0x246
  github.com/pingcap/check.(*suiteRunner).doRun()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:666 +0x184
  github.com/pingcap/check.(*suiteRunner).run()
      /nfs/cache/mod/github.com/pingcap/[email protected]/check.go:696 +0x1ee
  github.com/pingcap/check.Run()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:150 +0xa4
  github.com/pingcap/check.RunAll()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:117 +0xad3
  github.com/pingcap/check.TestingT()
      /nfs/cache/mod/github.com/pingcap/[email protected]/run.go:99 +0xb08
  github.com/pingcap/dm/pkg/election.TestSuite()
      /home/jenkins/agent/workspace/atom-ut/dm/pkg/election/election_test.go:39 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1193 +0x2a4
==================

this race on wg.Add golang/go#23842

Ehco1996 · 2021-09-30T08:05:27Z

/verify

Ehco1996 · 2021-09-30T08:34:16Z

fix testMaster.TestAgentPool in e3aae85

glorv · 2021-09-30T08:48:51Z

dm/master/election_test.go

@@ -99,4 +99,6 @@ func (t *testElectionSuite) TestFailToStartLeader(c *check.C) {
 	_, leaderID, _, err = s2.election.LeaderInfo(ctx)
 	c.Assert(err, check.IsNil)
 	c.Assert(leaderID, check.Equals, cfg2.Name)
+
+	cancel()


This cancal is redundant, already called defer cancel() 🤔

Actually no, the current test case relies on ctx.cancel() to exit and if ctx doesn't cancel, the test will never end (dm-master's server is keep running in backgroud)🤣

means upper defer stack relies on context cancel

Ehco1996 · 2021-09-30T12:43:58Z

/merge

ti-chi-bot · 2021-09-30T12:44:01Z

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

Commit hash: e3aae85

ti-chi-bot · 2021-09-30T12:45:23Z

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

…2191)

test: fix ctx not cancel in some ut

d7334cb

ti-chi-bot requested review from GMHDBJD and lance6716 September 30, 2021 02:31

ti-chi-bot added the size/S label Sep 30, 2021

okJiang reviewed Sep 30, 2021

View reviewed changes

dm/master/election_test.go Show resolved Hide resolved

fix more

a38581f

Ehco1996 changed the title ~~test: fix ctx not cancel in some ut~~ test: fix ctx timeout in advance before test success in ut Sep 30, 2021

okJiang approved these changes Sep 30, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 One reviewer already commented LGTM label Sep 30, 2021

fix TestElection2After1

69cf5ca

incr testPool timeout

e3aae85

glorv reviewed Sep 30, 2021

View reviewed changes

lance6716 approved these changes Sep 30, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Sep 30, 2021

Ehco1996 added the needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 label Sep 30, 2021

ti-chi-bot added the status/can-merge label Sep 30, 2021

ti-chi-bot merged commit 78c468d into pingcap:master Sep 30, 2021

ti-chi-bot mentioned this pull request Sep 30, 2021

test: fix ctx timeout in advance before test success in ut (#2189) #2191

Merged

ti-chi-bot added a commit that referenced this pull request Oct 8, 2021

test: fix ctx timeout in advance before test success in ut (#2189) (#…

7b237ee

…2191)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: fix ctx timeout in advance before test success in ut #2189

test: fix ctx timeout in advance before test success in ut #2189

Ehco1996 commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021 •

edited

Loading

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

glorv Sep 30, 2021

Ehco1996 Sep 30, 2021

lance6716 Sep 30, 2021

Ehco1996 commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021

test: fix ctx timeout in advance before test success in ut #2189

test: fix ctx timeout in advance before test success in ut #2189

Conversation

Ehco1996 commented Sep 30, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

ti-chi-bot commented Sep 30, 2021 • edited Loading

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

Ehco1996 commented Sep 30, 2021

glorv Sep 30, 2021

Choose a reason for hiding this comment

Ehco1996 Sep 30, 2021

Choose a reason for hiding this comment

lance6716 Sep 30, 2021

Choose a reason for hiding this comment

Ehco1996 commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021

ti-chi-bot commented Sep 30, 2021 •

edited

Loading