Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Reduce subsystem channel size on test networks #5436

Closed
eskimor opened this issue May 2, 2022 · 7 comments · Fixed by #5454
Closed

Reduce subsystem channel size on test networks #5436

eskimor opened this issue May 2, 2022 · 7 comments · Fixed by #5454
Assignees
Labels
T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes.

Comments

@eskimor
Copy link
Member

eskimor commented May 2, 2022

This way we can detect issues due to blocking channels on test networks better.

Given that the bounded channel size for subsystems seems to get passed into the overseer-gen macro a command line argument might be a bit too much to ask.

I'd suggest to use a feature flag that gets enabled on CI, but not for proper releases (we already had a similar setup in the past).

@drahnr
Copy link
Contributor

drahnr commented May 4, 2022

if we want this for a one-off test, we could just use the annotation #[overlord(.., message_capacity=16, ..)] and that's it. If you'd want this to be dynamic, that's definitely do-able, just requires maybe one or two more additions to the builder pattern.

@eskimor
Copy link
Member Author

eskimor commented May 4, 2022

Let's keep things simple. A feature flag that can be enabled on CI, but not for releases should already do the trick.

@drahnr
Copy link
Contributor

drahnr commented May 4, 2022

My formulation might have been unclear, the above is already impl'd and works :)

@drahnr drahnr closed this as completed May 4, 2022
@drahnr drahnr self-assigned this May 4, 2022
@eskimor
Copy link
Member Author

eskimor commented May 4, 2022

Wait, the feature flag itself does not exist yet - right? I will take care of that, but too soon to close then.

@eskimor eskimor reopened this May 4, 2022
@drahnr
Copy link
Contributor

drahnr commented May 4, 2022

That's correct, no cargo feature flag, but nothing todo and for a one-off, just creating a PR with the above changed setting might be enough.

@eskimor
Copy link
Member Author

eskimor commented May 4, 2022

I would expect us to deploy this on a couple of nodes - fix any issues and then have it deployed by default on test nets, like Versi. Hence the feature flag.

@drahnr
Copy link
Contributor

drahnr commented May 4, 2022

I'll create a lil smith tomorrow

drahnr added a commit that referenced this issue May 5, 2022
drahnr added a commit that referenced this issue May 5, 2022
@eskimor eskimor closed this as completed May 5, 2022
drahnr added a commit that referenced this issue May 5, 2022
* allow runtime adjustment of signal channel size

Closes #5436
@ordian ordian moved this to Ready for Review in Parachains-core Aug 16, 2022
@ordian ordian added the T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes. label Aug 16, 2022
@ordian ordian moved this from Ready for Review to Implemented in Parachains-core Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants