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

Flush expired transactions from mempool #2622

Closed
Tracked by #2309
mpguerra opened this issue Aug 13, 2021 · 5 comments · Fixed by #2774
Closed
Tracked by #2309

Flush expired transactions from mempool #2622

mpguerra opened this issue Aug 13, 2021 · 5 comments · Fixed by #2774
Assignees
Labels
C-enhancement Category: This is an improvement

Comments

@mpguerra
Copy link
Contributor

mpguerra commented Aug 13, 2021

Motivation

Specifications

Designs

Related Work

@mpguerra mpguerra mentioned this issue Aug 13, 2021
59 tasks
@mpguerra mpguerra added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Aug 13, 2021
@mpguerra mpguerra added this to the 2021 Sprint 19 milestone Aug 26, 2021
@mpguerra
Copy link
Contributor Author

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 7, 2021
@oxarbitrage
Copy link
Contributor

Need #2749

@oxarbitrage
Copy link
Contributor

oxarbitrage commented Sep 11, 2021

I have two questions in order to do this.

  1. Where we should do it? We need a place that is executed often so we can remove transactions that are expired. One option is in the crawler. We can add a mempool service argument in https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/commands/start.rs#L104 . This will mean we need to develop a mempool service request to remove transactions that will use the function added at Add mempool::Storage::remove() #2749 in the background. Any other ideas on where to do expired tx removal ?

  2. At some point we will have a transaction timestamp. Should we just compare this with the system clock ?

Thanks.

@oxarbitrage
Copy link
Contributor

@oxarbitrage
Copy link
Contributor

  1. Where we should do it? We need a place that is executed often so we can remove transactions that are expired. One option is in the crawler. We can add a mempool service argument in https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/commands/start.rs#L104 . This will mean we need to develop a mempool service request to remove transactions that will use the function added at Add mempool::Storage::remove() #2749 in the background. Any other ideas on where to do expired tx removal ?

One idea that was suggested by @conradoplg in discord was to add this in the mempool service poll_next():

"I think we can do this in the mempool poll_ready since it will get called periodically. At least the current code is relying on that to periodically check if transactions have been downloaded and adding them to the mempool if they were."

The implementation at #2774 uses the above.

There was another suggestion from @jvff however i was not able to code up this one:

"Idea: It could be a separate task spawned when the mempool is created, that:

loop {
    select! {
        _ = sleep_until(self.next_expiration()) => {
            self.remove_expired_transactions();
        }
        _ = next_expiration_rescheduled.next() => {}
    }
}

where next_expiration_rescheduled is an mpsc channel receiver whose sender is owned by the mempool::Storage and messages are sent when new transactions are inserted"

  1. At some point we will have a transaction timestamp. Should we just compare this with the system clock ?

After more research this question is not valid anymore, there is no timestamp involved in the mempool transaction expirations but just block heights.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants