Skip to content
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

Add --no-restart flag #65

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

CriesofCarrots
Copy link
Contributor

I want to test things like booting from a specific snapshot, so it would be handy to have the old /net ability to turn off automatic node restarts. startup_scripts.rs already supports a --no-restart flag.

I'm open to discussion on the best way to do this. It would actually be nice to have this configurable per node, but it serves my purposes for now to have a single hammer. Wdyt?

@gregcusack
Copy link
Collaborator

i think what you have here works for an all encompassing flag.
In terms of configurability per node, would you want something like ... --num-validators 10 --no-restart-count 4 ... or something, where 4 of the validators will not restart on failure? We could also add a label to the validators, so we would know which validators are being run with --no-restart?

Or would you like something that applies --no-restart to bootstrap, validator, and rpc groups of nodes individually?

@CriesofCarrots
Copy link
Contributor Author

Or would you like something that applies --no-restart to bootstrap, validator, and rpc groups of nodes individually?

I have been pondering if, down the road, it makes sense to enable per-node configuration via a yaml file or something like that. Other configs could include amount to node stake and even software-version/build-type. The benefit of up-front configuration would be enabling setup that needs to happen at genesis for a heterogeneous cluster, like #66.

But given the capabilities already provided by the heterogeneous-cluster approach, I think an all-or-nothing --no-restart is actually fine for now.

@gregcusack
Copy link
Collaborator

yesss i think this is the right way to do it down the road. and to be honest, I probably should have taken the yaml approach for heterogeneous clusters from the start.

Copy link
Collaborator

@gregcusack gregcusack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! thank you!

@CriesofCarrots CriesofCarrots merged commit 2c1566b into anza-xyz:main Aug 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants