Mute denied nodes

[dvdplm]

When a node is not allowed to connect, send a Mute message to NodeConnector.

A node can be disallowed to connect if the chain is over-quota or if the whole chain is banned, i.e. on the denylist.

Also dials down the logging of latest finalized block from info to debug.

[dvdplm]

When I test this locally the nodes keep trying to reconnect every 5-ish seconds so there's plenty of churn here on this. Is there a way to disconnect the socket and trigger an error on the node side? I think (but not sure) that nodes keep trying on error but not forever, so if we could do that it might be a solution to the bandwidth consumption?

[maciejhirsz]

I was thinking about adding Muted state to ConnMultiplex (might need a rename there) to then stop parsing messages in StreamHandler::handle, but still keeping the connection open. There is no way to keep it open without reading messages into buffers in actix-web AFAIK, but not doing the deserialization should help a lot (not if we are limited on the bandwidth though).

[dvdplm]

So you're saying, instead of stopping the NodeConnector actor when muting a node, we'd change the state to ConnMultiplex::Muted? We'd still need the conn_id/msg.id() to be able to look it up in the self.multiplex, but maybe you're thinking of using something non-serde to look up that key and avoid deserializing the full NodeMessage?

Tell me though, is the gain you're thinking about that it's cheaper to keep the node connected but not deserialize any of their messages than closing the actor and create a new one ever n seconds when they reconnect?

[dvdplm]

I tried the following hack: I added a muted: bool to the NodeConnector actor and instead of muting a node with ctx.close(None); ctx.stop(); I did self.muted = true and then changed the StreahHandler::handle to check this boolean as the first thing.
That would keep 1 NodeConnector actor around for every node but not process any of their messages. It feels pretty dirty and I'm not convinced it's better to keep the node connected rather than recreating them, but maybe you're right and that this is cheaper.

It feels to me like the right way to do this is for telemetry to close the connection with a proper reason and then change substrate to behave properly:

• When a chain is denied, telemetry closes the connection with a CloseCode::Policy (1008) (or possibly CloseCode::Abnormal (1006)); substrate does not try to reconnect.
• When a chain is over quota, telemetry closes the connection with a CloseCode::Again (1013) and the node is free to try to reconnect later (maybe with exponential backoff if we want to get fancy).

That seems like the only way we have to reduce both processing resources and bandwidth. Wdyt?

[maciejhirsz]

Grumbles and a suggestion how to "mute" the connector.

[maciejhirsz]

I was thinking about adding Muted state to ConnMultiplex (might need a rename there) to then stop parsing messages in StreamHandler::handle, but still keeping the connection open. There is no way to keep it open without reading messages into buffers in actix-web AFAIK, but not doing the deserialization should help a lot (not if we are limited on the bandwidth though).