Skip to content

Split mux's Tracer type #5112

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

Open
wants to merge 32 commits into
base: mwojtowicz/inbound-governor-turbo
Choose a base branch
from

Conversation

coot
Copy link
Contributor

@coot coot commented Apr 18, 2025

  • network-mux: removed handshake traces
  • network-mux: split mux tracer into three independent parts

Profiling exposed that firstPeerTo* are CPU intensive in the amount
of state checking performed for all active connections. Especially
the LastToFinish actions are expensive. However, much of this work
can be short-circuited. Addition of this tracer allows responding
to events in the order they arrive for just the pertinent
connection/peer, avoid repetitive state checking.

This tracer uses the information channel plumbed from the top level,
and in turn it itself is passed to the muxer by the connection handler
via the connection manager. The tracer is joined with the mux tracer
in the inbound handler, and outbound handler strictly when a duplex
connection is negotiated for the latter. In effect, the muxer has
a direct write-only communication channel to the inbound governor
where only a few of the events need to be monitored such that the IG
can properly manage its transitions.

Add IG tracer responder counters

Change RemoteIdle tag

The change is motivated by the need of the new tracer
to ensure proper sequencing of events on the queue. In case
a connection is expired and responder startup is demanded, the tracer
will retry until the connection is RemoteCold to register the
promotion, or abort when the connection is dropped (CM CommitTr returned).
Added responder counters state variable managed by the new tracer

Instantiate the connection handler with the tracer and run
the connection manager continuation with it. The IG tracer
is plumbed in the end to mux.
Moved 'InboundGovernorInfoChannel` to InboundGovernor
Previously, the queue was only used to communicate new connections
to the inbound governor. The queue is now used to also notify
the IG of muxer events so it will be busier.
Process all the info channel events from the queue in one step
of the IG loop. The queue events arrive from the CM (new connection)
or from the tracer which tracks miniprotocol responder activity
and mux start/stop.
Also reflects changes to 'RemoteIdle' tag
Most of the functionality of the Event is unneeded anymore
and the module is removed, with its still useful remnant moved
here. Updates reflecting changes to 'RemoteIdle' tag were applied.
The IG now applies the withConnectionManager continuation from the top
level Diffusion module to pass it the info channel tracer. Changes
in this module reflect the sequencing changes applied to the latter module.
Since the IG creates the connection handler by passing it
the new tracer, this argument was moved to the back of the list.
The prior arguments are created at the top level, where various
types are instantiated. This reflects the sequencing of applications
needed to create the CM.
Some comments, and reordering/addition of tracing which
reflects functional perspective of how the IG processes
responder events and mux start/stop events.

Fix termination bug

The inbound governor information channel can reach its limit
when the server is shutting down and the IG loop stops draining it.
The mux async exception handler writes to the queue when each
connection is being torn down by the CM, which then awaits for
each handler to complete.
Contains mux tracer and separate channel tracer. The idea
is that the channel tracer should not contain the inbound
governor's information channel tracer which may be present
in the muxTracer field for certain connections. The protocol channel send/receive
trace tags are uniteresting from the IG tracer's perspective
but there is a penalty for invoking it so frequently for
every complete message.
The tracer is attached to the inbound handler and to the outbound
handler only when a duplex mode was negotiated.
The bug was exposed when Mux.run signature was changed to accept
the new MuxTracerBundle type instead of the Mux.Trace's Trace type.
The latter Trace type mixes low level bearer tags with the higher level
mux tags, so logically they are separate and in fact are traced by
separate components. The headerTracer must go along with the bearer tracer
otherwise it doesn't trace anything, but the types matched so the
program was accepted.
The Server is started once the IG instantiates the CM with its tracer.
No other significant changes besides cosmetics.
This addresses the shrinker hang when attempting to shrink
'AbsBearerInfo' in some cases.
This change fixes a termination bug in command shrinking and improves
the quality of shrinker results for the 'DiffusionScript'. The
overall perfomance should also improve with better pruning of nodes
and the commands which direct their execution.

o-n-framework shrinker improvement
This annotes each trace with a nice (node-x), where x is a number
[1..max-node in simulation]. This facilitates grepping a trace
for just the node that failed a test.

In the mkTracers binding, one can either turn on all component tracers
for general testing, and in case of failure of a test where only a
subset of components are really needed, one can selectively toggle
just the interesting sayTracers to further cut down the noise.
Comprehensibility improvements
@coot coot added the mux issues related to network-mux label Apr 18, 2025
@coot coot requested a review from crocodile-dentist April 18, 2025 07:51
@coot coot self-assigned this Apr 18, 2025
@coot coot requested a review from a team as a code owner April 18, 2025 07:51
@github-project-automation github-project-automation bot moved this to In Progress in Ouroboros Network Apr 18, 2025
The `BearerTracer` has an extra event: `EmitDeltaQ`.

The tracers are now conveniently passed to `Mx.run` function from where
they are available to a channel & a bearer.
@coot coot force-pushed the coot/inbound-governor-turbo branch from b016212 to 1cbdaa2 Compare April 18, 2025 08:01
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/inbound-governor-turbo branch 6 times, most recently from 0f9e65c to 025e2d0 Compare April 25, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mux issues related to network-mux
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants