Skip to content

Commit

Permalink
Fix data race of outlier unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
wooyang2018 committed Oct 14, 2024
1 parent af130d0 commit a9eb7d0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
16 changes: 11 additions & 5 deletions core/outlier/recycler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ func newRecycler(resource string, interval time.Duration) *Recycler {
}
}

func (r *Recycler) length() int {
r.mtx.Lock()
defer r.mtx.Unlock()
return len(r.status)
}

func addOutlierRuleForRecycler(resource string, seconds uint32) {
updateMux.Lock()
defer updateMux.Unlock()
Expand Down Expand Up @@ -66,7 +72,7 @@ func testRecycler(t *testing.T) {

recycler := newRecycler(resource, 4*time.Second)
recycler.scheduleNodes(nodes)
assert.Equal(t, len(nodes), len(recycler.status))
assert.Equal(t, len(nodes), recycler.length())

// Restore node0 after 2 seconds of simulation
time.AfterFunc(2*time.Second, func() {
Expand Down Expand Up @@ -101,9 +107,9 @@ func testRecyclerConcurrent(t *testing.T) {
wg.Wait()

// Check the status of nodes
assert.GreaterOrEqual(t, len(nodes), len(recycler.status))
assert.GreaterOrEqual(t, len(nodes), recycler.length())
recycler.scheduleNodes(nodes)
assert.Equal(t, len(nodes), len(recycler.status))
assert.Equal(t, len(nodes), recycler.length())

// Recover node0 and node1 after 2 seconds
time.AfterFunc(2*time.Second, func() {
Expand Down Expand Up @@ -162,9 +168,9 @@ func testRecyclerChConcurrent(t *testing.T) {
wg.Wait()

// Check the status of nodes
assert.GreaterOrEqual(t, len(nodes), len(recycler.status))
assert.GreaterOrEqual(t, len(nodes), recycler.length())
recycler.scheduleNodes(nodes)
assert.Equal(t, len(nodes), len(recycler.status))
assert.Equal(t, len(nodes), recycler.length())

// Recover node0 and node1 after 2 seconds
time.AfterFunc(2*time.Second, func() {
Expand Down
20 changes: 12 additions & 8 deletions core/outlier/retryer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ func (d *dummyCall) Check(address string) bool {
}
return false
}
fmt.Println(d.callCounts)
panic("Attempting to call an unregistered node address.")
return false
}

func (d *dummyCall) getRecoverCount() int {
Expand Down Expand Up @@ -131,6 +129,12 @@ func setNodeBreaker(resource string, node string, breaker *MockCircuitBreaker) {
nodeBreakers[resource][node] = breaker
}

func (r *Retryer) length() int {
r.mtx.Lock()
defer r.mtx.Unlock()
return len(r.counts)
}

// Construct two dummy node addresses: the first one recovers after the third check,
// and the second one recovers after math.MaxInt32 checks. Observe the changes in the
// circuit breaker and callCounts status for the first node before and after recovery.
Expand All @@ -154,7 +158,7 @@ func testRetryer(t *testing.T) {
for d.getRecoverCount() < 1 {
time.Sleep(minDuration)
}
assert.Equal(t, len(nodes)-1, len(retryer.counts))
assert.Equal(t, len(nodes)-1, retryer.length())
mockCB.AssertExpectations(t)
}

Expand All @@ -175,7 +179,7 @@ func testRetryerConcurrent(t *testing.T) {
d.registerAddress(node, math.MaxInt32)
}
}
fmt.Println(d.callCounts)

addOutlierRuleForRetryer(resource, n, internal, d.Check)
retryer := getRetryerOfResource(resource)
numGoroutines := 10
Expand All @@ -194,15 +198,15 @@ func testRetryerConcurrent(t *testing.T) {
wg.Wait()

// Check the status of nodes
assert.GreaterOrEqual(t, len(nodes), len(retryer.counts))
assert.GreaterOrEqual(t, len(nodes), retryer.length())
retryer.scheduleNodes(nodes)
assert.Equal(t, len(nodes), len(retryer.counts))
assert.Equal(t, len(nodes), retryer.length())

minDuration := time.Duration(n * (n + 1) / 2 * internal * 1e6)
for d.getRecoverCount() < len(nodes)/2 {
time.Sleep(minDuration)
}
assert.Equal(t, len(nodes)/2, len(retryer.counts))
assert.Equal(t, len(nodes)/2, retryer.length())
for _, breaker := range mockCBs {
breaker.AssertExpectations(t)
}
Expand All @@ -229,7 +233,7 @@ func testRetryerCh(t *testing.T) {
for d.getRecoverCount() < 1 {
time.Sleep(minDuration)
}
assert.Equal(t, len(nodes)-1, len(retryer.counts))
assert.Equal(t, len(nodes)-1, retryer.length())
mockCB.AssertExpectations(t)
}

Expand Down

0 comments on commit a9eb7d0

Please sign in to comment.