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: search for a snapshot that fulfills a queried BlockHash or TxHash #5373

Merged
merged 62 commits into from
Nov 15, 2023

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Nov 9, 2023

PR into #5287

There are some changes to accomplish what the title says.

  • Snapshot file name now has transaction range as well. This helps in creating two inner indexes (BlockNumber -> Range<TxNumber> & TxNumber -> Range<BlockNumber>), where each key is the end number of a snapshot segment.
  • The above indexes are calculated when SnapshotProvider is created and read from disk. This enables handling custom range snapshots instead of being tied to the default one.
    • Given that, two functions were introduced: get_segment_provider_from_transaction and get_segment_provider_from_block
  • Adds SnapshotProvider::find_snapshot. This will iterate all existing snapshots (loading them from disk into the dashmap,) until we find one that returns Some(T). Example:
    fn header_td(&self, block_hash: &BlockHash) -> RethResult<Option<U256>> {
        self.find_snapshot(SnapshotSegment::Headers, |jar_provider| {
            Ok(jar_provider
                .cursor()?
                .get_two::<HeaderMask<CompactU256, BlockHash>>(block_hash.into())?
                .and_then(|(td, hash)| (&hash == block_hash).then_some(td.0)))
        })
  • Uses above to implement some more provider functions to SnapshotsProvider

@joshieDo joshieDo added C-enhancement New feature or request A-static-files Related to static files labels Nov 9, 2023
@joshieDo joshieDo requested review from shekhirin and mattsse November 9, 2023 21:45
@joshieDo joshieDo requested a review from rakita as a code owner November 9, 2023 21:45
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from joshie/db-snap-provider3 to main November 14, 2023 21:48
// Otherwise, check `self.available_snapshots`
let snapshot_ranges = match path {
Some(path) => SnapshotSegment::parse_filename(
path.file_name().ok_or_else(|| ProviderError::MissingSnapshot)?,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
path.file_name().ok_or_else(|| ProviderError::MissingSnapshot)?,
path.file_name().ok_or_else(|| ProviderError::MissingSnapshot(segment, path))?,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ended up creating more variants


while let Some((block_end, tx_range)) = snapshots_rev_iter.next() {
let block_start =
snapshots_rev_iter.peek().map(|(block_end, _)| *block_end + 1).unwrap_or(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we add a comment here why it's correct to do unwrap_or(0) and always return a range on latest snapshot?

@joshieDo joshieDo added this pull request to the merge queue Nov 15, 2023
Merged via the queue into main with commit a389a2b Nov 15, 2023
@joshieDo joshieDo deleted the joshie/db-snap-provider4 branch November 15, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-static-files Related to static files C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants