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

v2.1: add fanout to tpu-client-next (backport of #3478) #3523

Merged
merged 6 commits into from
Nov 25, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Nov 7, 2024

Problem

Crate tpu-client-next doesn't have fan out so far.

Summary of Changes

Add this feature.


This is an automatic backport of pull request #3478 done by [Mergify](https://mergify.com).

* Add tpu-client-next to the root Cargo.toml

* Change LeaderUpdater trait to accept mut self

* add fanout to the tpu-client-next

* Shutdown in separate task

* Use try_send instead, minor impromenets

* fix LeaderUpdaterError traits

* improve lifetimes in split_leaders

Co-authored-by: Illia Bobyr <[email protected]>

* address PR comments

* create connections in advance

* removed lookahead_slots

---------

Co-authored-by: Illia Bobyr <[email protected]>
(cherry picked from commit 2a618b5)

# Conflicts:
#	Cargo.toml
@mergify mergify bot requested a review from a team as a code owner November 7, 2024 16:47
@mergify mergify bot added the conflicts label Nov 7, 2024
Copy link
Author

mergify bot commented Nov 7, 2024

Cherry-pick of 2a618b5 has failed:

On branch mergify/bp/v2.1/pr-3478
Your branch is up to date with 'origin/v2.1'.

You are currently cherry-picking commit 2a618b5e01.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   tpu-client-next/src/connection_worker.rs
	modified:   tpu-client-next/src/connection_workers_scheduler.rs
	modified:   tpu-client-next/src/leader_updater.rs
	modified:   tpu-client-next/src/send_transaction_stats.rs
	modified:   tpu-client-next/src/workers_cache.rs
	modified:   tpu-client-next/tests/connection_workers_scheduler_test.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cargo.toml

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@alessandrod alessandrod self-requested a review November 13, 2024 13:11
alessandrod
alessandrod previously approved these changes Nov 13, 2024
@alessandrod
Copy link

I want to backport this because tpu-client-next is not used by anything yet (0 risk), but we're planning to make it opt-in in send-transaction-service (coming PRs). The existing client (before -next) creates stream concurrency and fragmentation. Both the agave and firedancer QUIC implementations now penalize concurrency, so we want to have a way to avoid it.

alessandrod
alessandrod previously approved these changes Nov 22, 2024
Copy link

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

This is probably a special case because it is all confined to the tpu-client-next crate that isn't actually used anywhere. But in the future, it might be better to split this up into multiple changes that could be reviewed and approved separately. For example, in addition to enabling fanout, we switch to try sends, atomic stats, looking ahead some number of leaders instead of slots, etc.

@alessandrod alessandrod merged commit 92f639b into v2.1 Nov 25, 2024
28 checks passed
@alessandrod alessandrod deleted the mergify/bp/v2.1/pr-3478 branch November 25, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants