Chorale: one room, one conductor — three ways the election could split - #151
Open
apirrone wants to merge 2 commits into
Open
Chorale: one room, one conductor — three ways the election could split#151apirrone wants to merge 2 commits into
apirrone wants to merge 2 commits into
Conversation
Reported from a four-duck room, `robotctl chorale` typed by hand seconds apart: sometimes it worked, sometimes two songs at once, sometimes nothing at all. Three separate bugs, and the guess in the report — "what if two ducks start singing before the other two have seen them?" — was the first one exactly. **Two conductors never resolved.** Nothing in `conduct` looked for a rival. Two ducks can pair off and start before either has seen the other two, who pair off and start their own piece — picked from their own clock, so not even the same song — and both performances then ran side by side to the end. Now a conductor that hears a *lower-id* duck conducting abandons its own performance and joins that one, so the room converges from any arrangement, by id, with nothing to negotiate and nothing to oscillate. **Deference had no timeout, which is the "nothing happens" case.** The lowest id conducts — but the lowest id *in one duck's view*, and BLE discovery is neither simultaneous nor symmetric. The lowest duck can still be seeing nobody while everybody else sees it, and a duck alone does not sing: three ducks then defer, forever, to a duck that will never start. Deference is now a timer — one extra SETTLE per lower-id duck in view — so the second-lowest takes over if the lowest never starts, in id order rather than three ducks starting three pieces at once. **A follower outlived its conductor.** It locks to one conductor by id and ignores every other beacon, which is what keeps a second piece from pulling it off the beat. The cost was that a conductor which stopped — yielded, or switched off — left its followers free-running to the end of a song nobody else was singing. Off the air or on the air and no longer singing now both mean: back to listening, and join whatever is actually being sung. The regression test is the reported room rather than a unit: four ducks, staggered starts, and a `visible` predicate that keeps them in two islands long enough to each elect a conductor. It asserts the split really happens, then that it ends with one conductor, the lower of the two, and all four on one piece. Each fix was checked by disabling it — the sim reports two conductors without the yield, and `[1, 2, 1, 1]` pieces without the follower fix.
Coverage70.59% lines on this branch, against a floor of 72%. Per-file |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from a four-duck room,
robotctl choraletyped by hand seconds apart: sometimes it worked, sometimes two songs at once, sometimes nothing at all. Three separate bugs, and the guess in the report — "what if two ducks start singing before the other two have seen them?" — was the first one exactly.Two conductors never resolved. Nothing in
conductlooked for a rival. Two ducks can pair off and start before either has seen the other two, who pair off and start their own piece — picked from their own clock, so not even the same song — and both performances then ran side by side to the end. Now a conductor that hears a lower-id duck conducting abandons its own performance and joins that one, so the room converges from any arrangement, by id, with nothing to negotiate and nothing to oscillate.Deference had no timeout, which is the "nothing happens" case. The lowest id conducts — but the lowest id in one duck's view, and BLE discovery is neither simultaneous nor symmetric. The lowest duck can still be seeing nobody while everybody else sees it, and a duck alone does not sing: three ducks then defer, forever, to a duck that will never start. Deference is now a timer — one extra SETTLE per lower-id duck in view — so the second-lowest takes over if the lowest never starts, in id order rather than three ducks starting three pieces at once.
A follower outlived its conductor. It locks to one conductor by id and ignores every other beacon, which is what keeps a second piece from pulling it off the beat. The cost was that a conductor which stopped — yielded, or switched off — left its followers free-running to the end of a song nobody else was singing. Off the air or on the air and no longer singing now both mean: back to listening, and join whatever is actually being sung.
The regression test is the reported room rather than a unit: four ducks, staggered starts, and a
visiblepredicate that keeps them in two islands long enough to each elect a conductor. It asserts the split really happens, then that it ends with one conductor, the lower of the two, and all four on one piece. Each fix was checked by disabling it — the sim reports two conductors without the yield, and[1, 2, 1, 1]pieces without the follower fix.