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

pull messages out of mempool rather than pushing messages into engine #203

Merged
merged 7 commits into from
Jan 9, 2025

Conversation

aditiharini
Copy link
Contributor

@aditiharini aditiharini commented Jan 8, 2025

Pull messages out of the mempool and run validations at that point so all messages fed to the engine are likely to be valid.

@aditiharini aditiharini marked this pull request as ready for review January 8, 2025 21:23
src/mempool/mempool.rs Outdated Show resolved Hide resolved
src/mempool/mempool.rs Show resolved Hide resolved
src/storage/store/engine.rs Outdated Show resolved Hide resolved
error!("Unable to send message to engine: {}", err.to_string())
loop {
tokio::select! {
message = self.mempool_rx.recv() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Selecting directly here is dangerous, the mempool rx channel and overload the messages_request_rx channel, and make it so the request is never fulfilled if we get sufficient message traffic to the mempool.

It's better to loop using a tick and ensure that the messages request channel is processed first and always prioritized.

See

let deadline = Instant::now() + timeout;
loop {
let timeout = time::sleep_until(deadline);
select! {
_ = poll_interval.tick() => {
for an example of how we're done this elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made a change to specify biased in the select macro. This causes the macro to preference branches in order of specification. I think this is what we want here?

In this case, I don't really think we need a timeout because we will just poll indefinitely.

@aditiharini aditiharini merged commit 23fe464 into main Jan 9, 2025
2 checks passed
@aditiharini aditiharini deleted the convert-to-pull-based-mempool branch January 9, 2025 03:53
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