object diffusion: more client agency for graceful termination, and make server idleness explicit - #5431
object diffusion: more client agency for graceful termination, and make server idleness explicit#5431prednaz wants to merge 3 commits into
Conversation
tbagrel1
left a comment
There was a problem hiding this comment.
Looks very good to me! I've only left a couple of syntax/naming nitpicks.
Related note: I don't think the naming convention CanAwait/MustReply for the states is very clear (the message names are fine though). I know it originates from old issues, and I think it was good during prototyping to keep that terminology, but I think it could be a good time now to think about the names a bit more :) Let me know if you have ideas
| -> OutboundStObjectIds kind objectId object m a | ||
| SendMsgAwaitReply | ||
| :: m (OutboundStObjectIds ('StObjectIdsBlocking 'StMustReply) objectId object m a) | ||
| -> OutboundStObjectIds ('StObjectIdsBlocking 'StCanAwait) objectId object m a |
There was a problem hiding this comment.
Similarly here, is there any advantage of having a mega-state with two phases instead of two distinct states?
There was a problem hiding this comment.
OutboundStObjectIds ('StObjectIdsBlocking 'StMustReply) and OutboundStObjectIds ('StObjectIdsBlocking 'StCanAwait) are just as distinct of 2 types as 2 hypothetical types OutboundStObjectIdsBlockingMustReply and OutboundStObjectIdsBlockingCanAwait. so i do not think it is appropriate to associate OutboundStObjectIds ('StObjectIdsBlocking 'StMustReply) and OutboundStObjectIds ('StObjectIdsBlocking 'StCanAwait) with anything else than 2 distinct states.
the advantage of parametric polymorphism is that code that is oblivious to the distinction expressed by the parameter does not have to be duplicated. SendMsgReplyObjectIds 3 lines above is an example. without parametric polymorphism, we would need to duplicate it into SendMsgReplyObjectIdsNonBlocking, SendMsgReplyObjectIdsBlockingMustReply, and SendMsgReplyObjectIdsBlockingCanAwait. a more important example is sendNewContent defined by a user of this library.
There was a problem hiding this comment.
the advantage of parametric polymorphism is that code that is oblivious to the distinction expressed by the parameter does not have to be duplicated. SendMsgReplyObjectIds 3 lines above is an example. without parametric polymorphism, we would need to duplicate it into SendMsgReplyObjectIdsNonBlocking, SendMsgReplyObjectIdsBlockingMustReply, and SendMsgReplyObjectIdsBlockingCanAwait. a more important example is sendNewContent defined by a user of this library.
Thanks for the explanation. Would you mind adding this as docstring in the code? Thanks :)
if it originated from old issues, those were very old issues indeed because
i suspect that staying consistent with |
|
Alright, I'm fine with keeping this terminology, as long as we just explain in a comment where it originates from and what it means exactly :) |
introduce `MsgServerIdle` to regularly return agency to the client from an idle server so the client can gracefully terminate the protocol as needed for tweag/cardano-peras#187
split `StObjectIds Blocking` into 2 states to make idleness of the server explicit so the client can decide if it has caught up or not as needed for tweag/cardano-peras#144
42e3318 to
2de679a
Compare


Description
Supports IntersectMBO/ouroboros-consensus#2286.
1. Support graceful Object Diffusion termination
introduce
MsgServerIdleto regularly return agency to the client from an idle server so the client can gracefully terminate the protocol as needed for tweag/cardano-peras#1872. Make Object Diffusion server idleness explicit
split
StObjectIds Blockinginto 2 states to make idleness of the server explicit so the client can decide if it has caught up or not as needed for tweag/cardano-peras#1443. default
NoThunksinstance forOuroboros.Network.PerasSupport.PerasSupportChecklist
Quality
Maintenance
ouroboros-networkproject.