Skip to content

Commit

Permalink
Merge #1649
Browse files Browse the repository at this point in the history
1649: Disable the on-demand start of mini-protocol threads for now r=dcoutts a=dcoutts

Fixes issue #1646.

This is an alternative to #1648. This PR simply disables on-demand start. That PR tries to fix it. However this bit of code is in the middle of being refactored so it is simpler to disable it for now and include the fix in the refactoring.

The problem, we realised, is that which peer initiates the overall mux bearer is actually independent from whether protocol threads should be started on-demand or started eagerly. What it really depends on is which peer has agency in the initial protocol state. For most mini-protocols
it is the peer that initiated the bearer that has agency in the initial state, but that is not true for all protocols, and in particular for the TxSubmission protocol in ouroboros-network.

The solution will be to make the on-demand vs eager distinction independent of the initiator vs responder distinction, and have it be specified in the MuxMiniProtocol.

We missed this in the network-mux and ouroboros-network tests because we did not have bundles of mini-protocols with mixed initial agency. This should be corrected.

Co-authored-by: Duncan Coutts <[email protected]>
  • Loading branch information
iohk-bors[bot] and dcoutts authored Feb 15, 2020
2 parents 6b28f10 + b8325e0 commit 5f77e24
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion network-mux/src/Network/Mux.hs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ muxStart tracer (MuxApplication ptcls) bearer = do
: demuxerJob dispatchtbl
: catMaybes [ miniProtocolInitiatorJob cnt tq ptcl pix initQ respQ
| (pix, (ptcl, initQ, respQ)) <- zip [0..] ptcls' ]
++ catMaybes [ miniProtocolResponderJob cnt tq ptcl pix initQ respQ
| (pix, (ptcl, initQ, respQ)) <- zip [0..] ptcls' ]

respondertbl = setupResponderTable
[ miniProtocolResponderJob cnt tq ptcl pix initQ respQ
Expand Down Expand Up @@ -260,7 +262,15 @@ monitor :: forall m. (MonadSTM m, MonadAsync m, MonadMask m)
monitor tracer jobpool
(MiniProtocolDispatch _ dispatchtbl)
(MiniProtocolResponders respondertbl) =
go (Set.fromList . range . bounds $ respondertbl)
go Set.empty
-- TODO: for the moment on-demand starting is disabled and all mini-protocol
-- threads are started eagerly. Doing this properly involves distinguishing
-- between on-demand and eager mini-protocols, but doing so independently of
-- whether overall we established the bearer as an initiator or as a responder.
-- The on-demand vs eager distinction can be derived from which peer(s) have
-- agency in the initial state of each mini-protocol.
--
-- go (Set.fromList . range . bounds $ respondertbl)
where
-- To do this second job it needs to keep track of which responder protocol
-- threads are running.
Expand Down

0 comments on commit 5f77e24

Please sign in to comment.