Wait for the elections the server tests expect - #15
Conversation
TestServerPromoteSelf sleeps twice the election timeout and then requires the server to already be leader. The wait before a follower puts itself forward is randomised between one and two election timeouts, so on a busy machine the sleep can expire first and the server reads back as a candidate. It failed CI on master this way. TestServerRecoverFromPreviousLogAndConf has the same shape twice, and fails in both directions: too early and a server has not caught up, too late and a second election has added another no-op and taken it past the number the test names. Wait for the commit index to reach what each phase expects rather than for an interval that looks long enough, and treat it as a floor. Pinning it exactly pins how many elections the run happened to hold, which is not what either test is about. The servers are all caught up before any of them is stopped, since stopping the leader first leaves the rest with nothing to catch up from.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
chrislusf has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
CI on master is red on
TestServerPromoteSelf:The test starts a single server and sleeps
2 * testElectionTimeoutbefore requiring it to be leader:The server runs with the default election timeout, and a follower waits a randomised interval between one and two of those before putting itself forward — up to 300ms — against a 400ms sleep. Add scheduling delay on a loaded runner and the sleep expires with the server still a candidate.
TestServerRecoverFromPreviousLogAndConfhas the same shape twice, and it has taken CI red on two branches recently. It fails in both directions: too early and a server has not caught up ([6/17]), too late and a second election has added another no-op and carried it past the number the test names ([18/16]).Fix
Wait for the state each phase expects instead of for an interval that looks long enough.
The commit index is treated as a floor rather than an exact value. Requiring exactly 16 or 17 pins how many elections the run happened to hold, which is not what either test is checking — extra no-ops are a leader doing its job. Under-commit is still caught, by the deadline.
Two things this turned up that are worth stating, because the first version of the fix had both:
-count=5, where the original sleep-then-check does not.Verification
TestServerPromoteSelfandTestServerRecoverFromPreviousLogAndConfat-count=3, ten times over: no failures. Full suite clean three times.