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

Support tpu disable quic in net scripts #27039

Merged

Conversation

lijunwangs
Copy link
Contributor

@lijunwangs lijunwangs commented Aug 9, 2022

Support passing --tpu-disable-quic in validator launched by net scripts

CriesofCarrots
CriesofCarrots previously approved these changes Aug 9, 2022
@KirillLykov
Copy link
Contributor

KirillLykov commented Aug 10, 2022

I think bench-tps client requires option to enable quic as well, otherwise user will need to remember to enable it manually (./net.sh start -c bench-tps=1="--tpu-use-quic")

Another thing is that it looks like quic will become default soon and option will be tpu-disable-quic quic by default PR. So these options in script will probably require renaming.

@mergify mergify bot dismissed CriesofCarrots’s stale review August 11, 2022 00:33

Pull request has been modified.

@KirillLykov
Copy link
Contributor

@lijunwangs , looks like now QUIC is default ( #27194).
I think having option --tpu-disable-quic would be beneficial also for these scripts

@lijunwangs lijunwangs force-pushed the support_tpu_use_quic_in_net_scripts branch from 7848ec0 to 213513a Compare August 24, 2022 00:51
@lijunwangs lijunwangs changed the title Support tpu use quic in net scripts Support tpu disable quic in net scripts Aug 24, 2022
@lijunwangs
Copy link
Contributor Author

@lijunwangs , looks like now QUIC is default ( #27194).
I think having option --tpu-disable-quic would be beneficial also for these scripts

Yes, it is funny -- I turned it to tpu-disable-quic now

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Thanks for flopping! It looks like multinode-demo/validator.sh needs updating as well. Do you mind?

@lijunwangs
Copy link
Contributor Author

Thanks for flopping! It looks like multinode-demo/validator.sh needs updating as well. Do you mind?

Sure. I will take care of it. I am testing this. I found the cluster would not start up with --tpu-disable-quic turned on. Still debugging.

@lijunwangs
Copy link
Contributor Author

Thanks for flopping! It looks like multinode-demo/validator.sh needs updating as well. Do you mind?

Sure. I will take care of it. I am testing this. I found the cluster would not start up with --tpu-disable-quic turned on. Still debugging.

It turns out the missing change multinode-demo/validator.sh was causing the issue as the member validator was started with default quic and the bootstrap was started with quic disabled.

@lijunwangs lijunwangs merged commit ed463dd into solana-labs:master Aug 24, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Aug 25, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Aug 25, 2022
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.

None yet

3 participants