paritytech/orchestra

`CHANNEL_CAPACITY` is unused

Closed this issue · 9 comments

Since 0.0.5 I get the warning message CHANNEL_CAPACITY from generated code.

Indeed, this constant is no longer used as we use it in the builder implementation.

In the meantime, we also have signal_channel_capacity, is there any case if we want to have it per-subsystem specific? If yes, then we can unify all the code.

The signal capacity is identical per subsystem, so we should never come to the point that we need different queues, unless there is a subsystem that blocks on tasks for a very long time and does not process signals.

Yes, but if we can override message capacity per subsystem, why cannot we override a signal capacity then?

Signals should be homogenous across all subsystems, and should be processed with priority over messages iirc, under that assumption I'd consider it a bug in the Subsystem if its blocking incoming signal consumption for too long

Yes, but some subsystems might be slow, so signals sent to them might block other susbsystems, similarly to paritytech/polkadot#18.

This taps into paritytech/polkadot-sdk#933 whether that's ok or not or if we should split Signal Processing into its own stream entirely

But I see that that'd be a large refactor mostly in polkadot, which might not be desirable at this point. Maybe it could be done by introducing prioritization into prioritized-metered-channel. But I degress.

The proposed approach is fine by me, but we should slow down the duck tape fixes needed short term for some more principal led arch refactors

In fact, I also think that having both message_capacity and signal_capacity on per-subsystem and per-overseer levels is a more consistent approach.