mirror of
https://github.com/kemko/nomad.git
synced 2026-01-09 11:55:42 +03:00
test: fix race around block eval chans
Similar to previous commit, stop and change chans were being set and
accessed from different goroutines. Passing the chans on the stack
resolves the race.
Output from `go test -race -run 'Server_RPC$' in nomad/
```
==================
WARNING: DATA RACE
Write at 0x00c0002b4e10 by goroutine 63:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).Flush()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:648
+0x32a
github.com/hashicorp/nomad/nomad.(*BlockedEvals).SetEnabled()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:149
+0x12b
github.com/hashicorp/nomad/nomad.(*Server).revokeLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:721
+0x232
github.com/hashicorp/nomad/nomad.(*Server).leaderLoop()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:122
+0x95d
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72
+0x6c
Previous read at 0x00c0002b4e10 by goroutine 75:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).watchCapacity()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:483
+0xfe
Goroutine 63 (running) created at:
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:70
+0x269
Goroutine 75 (finished) created at:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).SetEnabled()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:141
+0xba
github.com/hashicorp/nomad/nomad.(*Server).establishLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:210
+0x392
github.com/hashicorp/nomad/nomad.(*Server).leaderLoop()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:117
+0x82e
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72
+0x6c
==================
==================
WARNING: DATA RACE
Write at 0x00c0002b4e50 by goroutine 63:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).Flush()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:649
+0x388
github.com/hashicorp/nomad/nomad.(*BlockedEvals).SetEnabled()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:149
+0x12b
github.com/hashicorp/nomad/nomad.(*Server).revokeLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:721
+0x232
github.com/hashicorp/nomad/nomad.(*Server).leaderLoop()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:122
+0x95d
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72
+0x6c
Previous read at 0x00c0002b4e50 by goroutine 77:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).prune()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:690
+0xae
Goroutine 63 (running) created at:
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:70
+0x269
Goroutine 77 (finished) created at:
github.com/hashicorp/nomad/nomad.(*BlockedEvals).SetEnabled()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/blocked_evals.go:142
+0xdc
github.com/hashicorp/nomad/nomad.(*Server).establishLeadership()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:210
+0x392
github.com/hashicorp/nomad/nomad.(*Server).leaderLoop()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:117
+0x82e
github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1()
/home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72
+0x6c
==================
```
This commit is contained in:
@@ -4,7 +4,7 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/armon/go-metrics"
|
||||
metrics "github.com/armon/go-metrics"
|
||||
"github.com/hashicorp/consul/lib"
|
||||
log "github.com/hashicorp/go-hclog"
|
||||
"github.com/hashicorp/nomad/helper"
|
||||
@@ -138,8 +138,8 @@ func (b *BlockedEvals) SetEnabled(enabled bool) {
|
||||
b.l.Unlock()
|
||||
return
|
||||
} else if enabled {
|
||||
go b.watchCapacity()
|
||||
go b.prune()
|
||||
go b.watchCapacity(b.stopCh, b.capacityChangeCh)
|
||||
go b.prune(b.stopCh)
|
||||
} else {
|
||||
close(b.stopCh)
|
||||
}
|
||||
@@ -475,12 +475,12 @@ func (b *BlockedEvals) UnblockClassAndQuota(class, quota string, index uint64) {
|
||||
|
||||
// watchCapacity is a long lived function that watches for capacity changes in
|
||||
// nodes and unblocks the correct set of evals.
|
||||
func (b *BlockedEvals) watchCapacity() {
|
||||
func (b *BlockedEvals) watchCapacity(stopCh <-chan struct{}, changeCh <-chan *capacityUpdate) {
|
||||
for {
|
||||
select {
|
||||
case <-b.stopCh:
|
||||
case <-stopCh:
|
||||
return
|
||||
case update := <-b.capacityChangeCh:
|
||||
case update := <-changeCh:
|
||||
b.unblock(update.computedClass, update.quotaChange, update.index)
|
||||
}
|
||||
}
|
||||
@@ -606,6 +606,11 @@ SCAN:
|
||||
b.l.Unlock()
|
||||
return dups
|
||||
}
|
||||
|
||||
// Capture chans inside the lock to prevent a race with them getting
|
||||
// reset in Flush
|
||||
dupCh := b.duplicateCh
|
||||
stopCh := b.stopCh
|
||||
b.l.Unlock()
|
||||
|
||||
// Create the timer
|
||||
@@ -616,11 +621,11 @@ SCAN:
|
||||
}
|
||||
|
||||
select {
|
||||
case <-b.stopCh:
|
||||
case <-stopCh:
|
||||
return nil
|
||||
case <-timeoutCh:
|
||||
return nil
|
||||
case <-b.duplicateCh:
|
||||
case <-dupCh:
|
||||
goto SCAN
|
||||
}
|
||||
}
|
||||
@@ -676,13 +681,13 @@ func (b *BlockedEvals) EmitStats(period time.Duration, stopCh chan struct{}) {
|
||||
}
|
||||
|
||||
// prune is a long lived function that prunes unnecessary objects on a timer.
|
||||
func (b *BlockedEvals) prune() {
|
||||
func (b *BlockedEvals) prune(stopCh <-chan struct{}) {
|
||||
ticker := time.NewTicker(pruneInterval)
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-b.stopCh:
|
||||
case <-stopCh:
|
||||
return
|
||||
case <-ticker.C:
|
||||
b.pruneUnblockIndexes()
|
||||
|
||||
Reference in New Issue
Block a user