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

feat(db): use bincode for starknet_transactions #1883

Merged

Conversation

sistemd
Copy link
Contributor

@sistemd sistemd commented Mar 15, 2024

At the same time as changing the format, I tried to optimize and clean up where I could. Let me know if I missed something.

Added DB versioning to the bincoded columns (tx, receipt, events).

This results in a 15GiB saving on Goerli, which is a 8.7% reduction. Maybe we should do this for the headers, too.

Last PR for #1843 before I can open the final big PR.

@sistemd sistemd requested a review from a team as a code owner March 15, 2024 00:34
@sistemd sistemd marked this pull request as draft March 15, 2024 00:44
@kkovaacs
Copy link
Contributor

Another idea: looks like the migration doesn't fully use the available I/O bandwidth (at least with an NVMe SSD) and it's CPU bound. I think we could do the decompression/JSON parsing/bincode encoding/compression steps concurrently so that we use all available I/O bandwidth?

The mainnet migration has been running for >2 hours for me and it's still busy re-encoding stuff (so not yet at DROP TABLE). Maybe we could bring this down considerably?

@sistemd
Copy link
Contributor Author

sistemd commented Mar 20, 2024

Note to self: don't store transaction hashes in DTOs since they're already stored as a column. Will be done is part of follow-up.

@sistemd sistemd marked this pull request as ready for review March 21, 2024 12:00
Copy link
Contributor

@kkovaacs kkovaacs left a comment

Choose a reason for hiding this comment

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

LGTM, but looks like you'll have to rebase to latest main because there are conflicts (including a trivial one because of a storage migration).

Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for persevering through all my only-sometimes-applicable comments!

@sistemd sistemd force-pushed the sistemd/bincode-instead-of-json branch 2 times, most recently from 70f571c to f46d11a Compare March 25, 2024 08:22
@sistemd sistemd merged commit cbe4de5 into sistemd/1843-split-receipts-and-events Mar 25, 2024
12 of 14 checks passed
@sistemd sistemd deleted the sistemd/bincode-instead-of-json branch March 25, 2024 09:09
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