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.
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
Simplify simulation messaging with priority queue and parameterise GST #209
Conversation
1bf3314
to
a0d96e5
Compare
a0d96e5
to
b4ef5a0
Compare
Use `heap.Interface` to simplify the implementation of message passing during simulation by implementing a priority queue. The messages in-flight are priorities in ascending order of their `deliverAt` time, i.e. messages with earlier delivery time have higher priority. Additionally, explicitly introduce an option to set Global Stabilization Time (GST). GST duration is one of the fundamental assumptions made by gPBFT, beyond which message delivery is assumed to be guaranteed. Enhance Withold adversary tests with varying GST time. Relates to #196
b4ef5a0
to
ee3c5e2
Compare
func (pq *messageQueue) Pop() any { | ||
old := pq.mailbox | ||
n := len(old) | ||
item := old[n-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's removing the lowest priority (highest deliverAt) item, if they are in ascending order. Which I don't think is what we want here.
This message queue is lacking tests (my fault).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of Pop
is correct, i.e. adheres to what heap.Interface
demands. It's my documentation that's misleading; apologies.
Unit tests are added here along with documentation correction.
if adv != nil && !n.globalStabilisationElapsed { | ||
if n.hasGlobalStabilizationTimeElapsed() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit confusing to read. We should just remove the boolean field if this can be calculated instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the boolean filed to cache that GST has elapsed. Because, otherwise the simulation logs "GST elapsed" on every tick after GST has elapsed.
If you are happy for the log line to be removed then we can totally get rid of the boolean flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok makes sense, thanks.
Assert that simulation priority queue used to order message delivery is behaving as expected: * queued messages are sorted in ascending order of their `deliverAt`. * removal of a message returns the one with earliest `deliverAt`. * conditional upsert is respected. * duplicate messages are allowed and ordered as expected. * queue is fully drained upon removing all messages. Update misleading godoc for `Pop` receiver. Addresses #209 (comment)
Assert that simulation priority queue used to order message delivery is behaving as expected: * queued messages are sorted in ascending order of their `deliverAt`. * removal of a message returns the one with earliest `deliverAt`. * conditional upsert is respected. * duplicate messages are allowed and ordered as expected. * queue is fully drained upon removing all messages. Update misleading godoc for `Pop` receiver. Addresses #209 (comment)
Assert that simulation priority queue used to order message delivery is behaving as expected: * queued messages are sorted in ascending order of their `deliverAt`. * removal of a message returns the one with earliest `deliverAt`. * conditional upsert is respected. * duplicate messages are allowed and ordered as expected. * queue is fully drained upon removing all messages. Update misleading godoc for `Pop` receiver. Addresses #209 (comment)
Assert that simulation priority queue used to order message delivery is behaving as expected: * queued messages are sorted in ascending order of their `deliverAt`. * removal of a message returns the one with earliest `deliverAt`. * conditional upsert is respected. * duplicate messages are allowed and ordered as expected. * queue is fully drained upon removing all messages. Update misleading godoc for `Pop` receiver. Addresses #209 (comment)
Use
heap.Interface
to simplify the implementation of message passing during simulation by implementing a priority queue. The messages in-flight are priorities in ascending order of theirdeliverAt
time, i.e. messages with earlier delivery time have higher priority.Additionally, explicitly introduce an option to set Global Stabilization Time (GST). GST duration is one of the fundamental assumptions made by gPBFT, beyond which message delivery is assumed to be guaranteed. Enhance Withold adversary tests with varying GST time.
Relates to #196