Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix the peergrouper intermittent failing test. #6833
Conversation
QARan the individual test, and the entire suite in a loop with -race. Before the fix the test would fail ~33% of the time. Cannot get it to fail now. |
| @@ -128,7 +128,7 @@ func possiblePeerGroupChanges( | ||
| logger.Debugf("machine %q is a potential voter", m.Id()) | ||
| toAddVote = append(toAddVote, m) | ||
| } else { | ||
| - logger.Debugf("machine %q is not ready (has status: %v)", m.Id(), ok) | ||
| + logger.Debugf("machine %q is not ready (has status: %v, healthy: %v)", m.Id(), status.State, status.Healthy) |
mjs
Jan 19, 2017
Contributor
minor tweak: the "has" doesn't add anything and would make the output more concise if removed
| mustNext(c, memberWatcher) | ||
| assertMembers(c, memberWatcher.Value(), mkMembers("1v 2v 3v", ipVersion)) | ||
| + // Stop the clock advancing goroutine. | ||
| + close(done) |
mjs
Jan 19, 2017
Contributor
This should be a defer above or else the clock advancing goroutine ends up being left behind if the test fails.
| - c.Logf("mustNext done %p, ok: %v, val: %#v", w, ok, val) | ||
| + if ok { | ||
| + members := val.([]replicaset.Member) | ||
| + val = "\n" + prettyReplicaSetMembers(members) |
mjs
Jan 19, 2017
Contributor
This looks problematic! val is being updated to improve the log output but the modified val is also being returned via voyerResult. I'm pretty sure that's not intended.
howbazaar
Jan 19, 2017
Owner
Actually val is an interface{}, and output with %v, so this is just a convenience.
Given that this is a test, if the voyer result is not []replicaset.Member, it'll panic.
It is very much intended, and a convenience to output the voyer result if !ok.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
howbazaar commentedJan 19, 2017
Due to the number of watcher goroutines and the way in which the time based checking of the peer group works, there is no definitive controlled way to advance the testing clock such that it always works. So, don't do that.
Other parts of this PR relate to the output that is shown when the test runs. It is much easier to follow now.