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

[release-3.4] Races in TestLessorRenewExtendPileup TestLessorDetach #18538

Closed
serathius opened this issue Sep 4, 2024 · 5 comments · Fixed by #18555
Closed

[release-3.4] Races in TestLessorRenewExtendPileup TestLessorDetach #18538

serathius opened this issue Sep 4, 2024 · 5 comments · Fixed by #18555

Comments

@serathius
Copy link
Member

serathius commented Sep 4, 2024

Which Github Action / Prow Jobs are flaking?

Running make test-unit on release-3.4 branch detects races for me

Which tests are flaking?

TestLessorRenewExtendPileup and TestLessorDetach

Github Action / Prow Job link

No response

Reason for failure (if possible)

No response

Anything else we need to know?

No response

@serathius
Copy link
Member Author

Minimal repro go test ./lease/ --count 10 -v --race

=== RUN   TestLessorRenewExtendPileup
==================
WARNING: DATA RACE
Write at 0x00000143c590 by goroutine 385:
  go.etcd.io/etcd/lease.TestLessorRenewExtendPileup()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor_test.go:294 +0x289
  testing.tRunner()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1742 +0x44

Previous read at 0x00000143c590 by goroutine 188:
  go.etcd.io/etcd/lease.(*lessor).revokeExpiredLeases()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor.go:629 +0x3a
  go.etcd.io/etcd/lease.(*lessor).runLoop()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor.go:612 +0xe4
  go.etcd.io/etcd/lease.newLessor.gowrap1()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor.go:238 +0x33

Goroutine 385 (running) created at:
  testing.(*T).Run()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1742 +0x825
  testing.runTests.func1()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:2161 +0x85
  testing.tRunner()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1689 +0x21e
  testing.runTests()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:2159 +0x8be
  testing.(*M).Run()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:2027 +0xf17
  main.main()
      _testmain.go:93 +0x2bd

Goroutine 188 (running) created at:
  go.etcd.io/etcd/lease.newLessor()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor.go:238 +0x590
  go.etcd.io/etcd/lease.TestLessorCheckpointPersistenceAfterRestart.func1()
      /usr/local/google/home/siarkowicz/src/go.etcd.io/etcd/lease/lessor_test.go:722 +0x7c4
  testing.tRunner()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /usr/local/google/home/siarkowicz/.gvm/gos/go1.22/src/testing/testing.go:1742 +0x44
==================
    testing.go:1398: race detected during execution of test
--- FAIL: TestLessorRenewExtendPileup (0.06s)

@jmhbnz
Copy link
Member

jmhbnz commented Sep 8, 2024

I was also able to reproduce with go test ./lease/ --count 10 -v --race.

=== RUN   TestLessorRenewExtendPileup                                                                                         
==================                                                                                                            
WARNING: DATA RACE                                                                                                            
Write at 0x0000014595b0 by goroutine 377:                                                                                     
  go.etcd.io/etcd/lease.TestLessorRenewExtendPileup()                                                                         
      /home/james/Documents/etcd/lease/lessor_test.go:295 +0x289                                                              
  testing.tRunner()                                                                                                           
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1689 +0x21e                             
  testing.(*T).Run.gowrap1()                                                                                                  
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1742 +0x44                              
                                                                                                                              
Previous read at 0x0000014595b0 by goroutine 187:                                                                             
  go.etcd.io/etcd/lease.(*lessor).revokeExpiredLeases()                                                                       
      /home/james/Documents/etcd/lease/lessor.go:631 +0x3a                                                                    
  go.etcd.io/etcd/lease.(*lessor).runLoop()                                                                                   
      /home/james/Documents/etcd/lease/lessor.go:614 +0xe4                                                                    
  go.etcd.io/etcd/lease.newLessor.gowrap1()                                                                                   
      /home/james/Documents/etcd/lease/lessor.go:238 +0x33                                                                    
                                                                                                                              
Goroutine 377 (running) created at:                                                                                           
  testing.(*T).Run()                                                                                                          
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1742 +0x825                             
  testing.runTests.func1()                                                                                                    
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:2161 +0x85                              
  testing.tRunner()                                                                                                           
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1689 +0x21e                             
  testing.runTests()                                                                                                          
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:2159 +0x8be                             
  testing.(*M).Run()                                                                                                          
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:2027 +0xf17                             
  main.main()                                                                                                                 
      _testmain.go:93 +0x2bd                                                                                                  
                                                                                                                              
Goroutine 187 (running) created at:                                                                                           
  go.etcd.io/etcd/lease.newLessor()                                                                                           
      /home/james/Documents/etcd/lease/lessor.go:238 +0x590                                                                   
  go.etcd.io/etcd/lease.TestLessorCheckpointPersistenceAfterRestart.func1()                                                   
      /home/james/Documents/etcd/lease/lessor_test.go:724 +0x7c4                                                              
  testing.tRunner()                                                                                                           
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1689 +0x21e                             
  testing.(*T).Run.gowrap1()                                                                                                  
      /home/linuxbrew/.linuxbrew/Cellar/[email protected]/1.22.7/libexec/src/testing/testing.go:1742 +0x44                              
==================                                                                                                            
    testing.go:1398: race detected during execution of test                                                                   

@lucasrod16
Copy link
Contributor

Hi @serathius @jmhbnz,

I was able to reproduce on the main branch as well with:

cd server
go test ./lease/ --count 10 -v --race --failfast

I fixed the race condition and verified by running the unit tests locally.

I submitted a PR to fix: #18555

Looking forward to your feedback, thanks!

@jmhbnz
Copy link
Member

jmhbnz commented Sep 9, 2024

Closing - Backports completed, many thanks @lucasrod16 for tackling this one! 🙏🏻

@jmhbnz jmhbnz closed this as completed Sep 9, 2024
@lucasrod16
Copy link
Contributor

I appreciate the feedback and timely reviews! @jmhbnz @serathius @ahrtr

I have been interested in getting involved with etcd as a regular contributor and will continue to look for things to help with. I hope to be able to start attending the community and triage meetings as well.

Once again, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants