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

bolt: Improve delete inactive orders/matches perf. #3059

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

jholdstock
Copy link
Member

Running these functions on a resource contrained machine and a large database was incredibly slow due to overuse of the newestBuckets func. Every key was accessed and sorted every time a batch of orders/matches was processed.

Rather than repeatedly performing this work, the full set of keys is now accessed only once. The resulting code runs ~200x faster.

There is a slight change in behaviour because database transactions are now committed with a maximum of 1000 deletions rather than exactly 1000. This is not an issue because the exact size of each transaction doesn't matter, the important point is to limit the maximum size of each transaction to prevent excess memory consumption.

This performance gain removes the need to log the progress of each individual batch, so now only a summary is logged after the entire deletion process is completed.

Running these functions on a resource contrained machine and a large
database was incredibly slow due to overuse of the newestBuckets func.
Every key was accessed and sorted every time a batch of orders/matches
was processed.

Rather than repeatedly performing this work, the full set of keys is now
accessed only once. The resulting code runs ~200x faster.

There is a slight change in behaviour because database transactions are
now committed with a *maximum* of 1000 deletions rather than *exactly*
1000. This is not an issue because the exact size of each transaction
doesn't matter, the important point is to limit the maximum size of each
transaction to prevent excess memory consumption.

This performance gain removes the need to log the progress of each
individual batch, so now only a summary is logged after the entire
deletion process is completed.
@jholdstock
Copy link
Member Author

Just reviewed history and noticed @JoeGruffins was the original author of this code. Care to give it a once over Joe?

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

I can't remember writing this but changes look good and tests passing.

@buck54321 buck54321 merged commit 35fe334 into decred:master Nov 7, 2024
5 checks passed
buck54321 pushed a commit to buck54321/dcrdex that referenced this pull request Nov 12, 2024
Running these functions on a resource contrained machine and a large
database was incredibly slow due to overuse of the newestBuckets func.
Every key was accessed and sorted every time a batch of orders/matches
was processed.

Rather than repeatedly performing this work, the full set of keys is now
accessed only once. The resulting code runs ~200x faster.

There is a slight change in behaviour because database transactions are
now committed with a *maximum* of 1000 deletions rather than *exactly*
1000. This is not an issue because the exact size of each transaction
doesn't matter, the important point is to limit the maximum size of each
transaction to prevent excess memory consumption.

This performance gain removes the need to log the progress of each
individual batch, so now only a summary is logged after the entire
deletion process is completed.
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.

4 participants