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

Define BankingTracer::create_channels() #4041

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

ryoqun
Copy link
Member

@ryoqun ryoqun commented Dec 10, 2024

Problem

unified scheduler wants to treat all the transactions as same.

Summary of Changes

Unify the incoming channels for the banking stage conditionally. Currently stubbed.

No functional change, so no test is added.

extracted from #3946, see the pr for the general overview.

self.create_channel(ChannelLabel::TpuVote)
}

pub fn create_channel_gossip_vote(&self) -> (BankingPacketSender, BankingPacketReceiver) {
fn create_channel_gossip_vote(&self) -> (BankingPacketSender, BankingPacketReceiver) {
self.create_channel(ChannelLabel::GossipVote)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

these fns are now private

@ryoqun ryoqun requested a review from apfitzge December 10, 2024 15:18
@ryoqun
Copy link
Member Author

ryoqun commented Dec 12, 2024

@apfitzge ping :)

@@ -220,22 +230,85 @@ impl BankingTracer {
self.active_tracer.is_some()
}

pub fn create_channels(&self, pool: Option<&Arc<DefaultSchedulerPool>>) -> Channels {

Choose a reason for hiding this comment

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

neat - I actually wanted to experiment with this in central-scheduler as well. Now I don't have to do the work 😄

Choose a reason for hiding this comment

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

given that, can we just make this take a bool?
unified-scheduler can have this always be true (Option::is_some) and for central-scheduler can control via some sort of configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

done: 1d01404

Copy link

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

@apfitzge
Copy link

@apfitzge ping :)

ha. was literally typing my comments as you pinged me 😆

@ryoqun ryoqun requested a review from apfitzge December 12, 2024 15:46
@ryoqun ryoqun merged commit d11072e into anza-xyz:master Dec 13, 2024
40 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.

2 participants