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.

Sign up for GitHub

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

Jump to bottom

Fix fake event recorder race #26619

Merged
merged 1 commit into from
Jun 1, 2016
Merged

Fix fake event recorder race #26619

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 7 additions & 4 deletions pkg/controller/persistentvolume/framework_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,14 @@ func (r *volumeReactor) checkClaims(t *testing.T, expectedClaims []*api.Persiste
func checkEvents(t *testing.T, expectedEvents []string, ctrl *PersistentVolumeController) error {
var err error

// Read recorded events
// Read recorded events - wait up to 1 minute to get all the expected ones
// (just in case some goroutines are slower with writing)
timer := time.NewTimer(time.Minute)

fakeRecorder := ctrl.eventRecorder.(*record.FakeRecorder)
gotEvents := []string{}
finished := false
for !finished {
for len(gotEvents) < len(expectedEvents) && !finished {
select {
case event, ok := <-fakeRecorder.Events:
if ok {
Expand All @@ -372,8 +375,8 @@ func checkEvents(t *testing.T, expectedEvents []string, ctrl *PersistentVolumeCo
glog.V(5).Infof("event recorder finished")
finished = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entering for loop condition is &, so what is the difference between the old version and new one if the path comes to this point? After finished is set true at this line, it will quit waiting, I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, when finished is true the loop exits. But finished is set at different condition now.

Before it was set when there was no event in channel. This has proven to be racy, I got the controller waiting for CPU to send an event and the event was missed in this loop. Now we wait 1 minute and only after that finished is set.

Of course, when sender closes the channel, finished is set immediately in both cases.

}
default:
glog.V(5).Infof("event recorder finished")
case _, _ = <-timer.C:
glog.V(5).Infof("event recorder timeout")
finished = true
}
}
Expand Down