Skip to content

Added option to specify tpu addresses in vortexor #6116

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

Merged
merged 4 commits into from
May 8, 2025

Conversation

lijunwangs
Copy link

Problem

Need to options to specify the TPU addresses in the vortexor so it is more deterministic to pair the vortexor and validator.

Summary of Changes

created the following options

--tpu-address
--tpu-forward-address

Also in the log show more friendly on how to pair the validator with the vortexor.

Fixes #

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Project coverage is 82.9%. Comparing base (0e43ebc) to head (99924ce).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6116   +/-   ##
=======================================
  Coverage    82.9%    82.9%           
=======================================
  Files         842      842           
  Lines      377667   377684   +17     
=======================================
+ Hits       313174   313192   +18     
+ Misses      64493    64492    -1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tpu_fwd_address.port().saturating_sub(QUIC_PORT_OFFSET),
);

for destination in destinations.read().unwrap().iter() {

Choose a reason for hiding this comment

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

This is great UX, thank you!

@@ -92,6 +92,19 @@ pub struct Cli {
#[arg(long, value_parser = parse_port_range, value_name = "MIN_PORT-MAX_PORT", default_value = get_default_port_range())]
pub dynamic_port_range: (u16, u16),

Choose a reason for hiding this comment

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

How many dynamic ports does vortexor need? Maybe it could be easier to just make tpu_address and tpu_forwards_address required arguments and remove the dynamic_port_range argument?

Copy link
Author

Choose a reason for hiding this comment

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

the dynamic_port_range is also used for client socket binds for rpc and websocket. We may add more sockets in the future such as for RPC service of the vortexor for subscription management.

Choose a reason for hiding this comment

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

Oh I see, yes in this case it makes sense to have it to deconflict with agave. Also please make sure that defaults here are not the same as for agave itself, so if this is not provided we do not start accidentally binding on top of agave's default port range=)

@lijunwangs lijunwangs force-pushed the option_to_set_tpu_addresses branch from d822eb7 to 274215e Compare May 7, 2025 20:37
@lijunwangs lijunwangs requested review from apfitzge and sakridge May 8, 2025 02:58
Copy link

@alexpyattaev alexpyattaev left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@lijunwangs lijunwangs merged commit 0142ceb into anza-xyz:master May 8, 2025
47 checks 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.

3 participants