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

Receipt key with block number #5651

Merged
merged 7 commits into from
May 8, 2023
Merged

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented May 4, 2023

  • Prefix receipt's data key with block number like erigon.
  • Reduced write amplification during sync. Total write goes down from 5TB to 2.65TB.
  • Does not noticably increase disk space use (in my run the one without blocknumber is actually slighly higher). This is likely because rocksdb have delta compression turned on by default for keys, so multiple keys with same prefix got compressed really well.
  • Unexpectedly did not improve RPC performance, nor does it reduce iops. I guess on SSDs at least, it just reduce write amplification. Maybe with readahead flag it'll do something. not even with readahead. I guess 50iops per receipts means something else is using it...
  • Tested on non-compact receipts as compact receipts still need to load blocks.

Screenshot from 2023-05-04 17-05-25

Changes

  • Address receipts by blocknum+hash.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • Scanned log with CL filter with archive node. Log matches.

@asdacap asdacap marked this pull request as ready for review May 4, 2023 11:05
@LukaszRozmej
Copy link
Member

I think to have performance improvement we should enable sorting in database

@asdacap
Copy link
Contributor Author

asdacap commented May 4, 2023

Sorting in database?

@LukaszRozmej
Copy link
Member

Was thinking about SetAdviseRandomOnOpen but it is probably wrong suggestion

@LukaszRozmej
Copy link
Member

I see one problem - this won't work without resync right? If someone just upgrades DB it will break?

@asdacap
Copy link
Contributor Author

asdacap commented May 4, 2023

No, it works fine. I can sync without it, then read with new change.

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

Overall cool.

Maybe we should decide the key scheme at the begining - read 1 key from blocks/receipts DB:

  • if it has prefix - use prefix scheme
  • if it doesn't have prefix or no DB - use old scheme

This way we would keep the existing databases consistent and avoid double read from existing databases.

@asdacap
Copy link
Contributor Author

asdacap commented May 5, 2023

Ok, but which key to read?

@LukaszRozmej
Copy link
Member

Ok, but which key to read?

whichever from block db? like open iterator get first key or something similar?

@asdacap asdacap requested a review from a team as a code owner May 8, 2023 01:27
@asdacap
Copy link
Contributor Author

asdacap commented May 8, 2023

Done

@asdacap asdacap merged commit fa44ee5 into master May 8, 2023
@asdacap asdacap deleted the perf/receipt-key-with-blocknumber branch May 8, 2023 13:06
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.

3 participants