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

Parallelise block/transaction changes creation in Importer #2618

Open
wants to merge 19 commits into
base: take_vec_changes_rocksdb
Choose a base branch
from

Conversation

AurelienFT
Copy link
Contributor

Description

Parallelise block/transaction changes creation in Importer. This can allow us to execute the transactions in the same time as the rest of the data needed to commit the block to the data is encoded in a Changes form.

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog
  • New behavior is reflected in tests
  • The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

@AurelienFT AurelienFT requested a review from a team January 22, 2025 13:35
@AurelienFT AurelienFT self-assigned this Jan 22, 2025
@AurelienFT AurelienFT added the xxx label Jan 22, 2025
@AurelienFT AurelienFT removed the request for review from a team January 22, 2025 14:14
@AurelienFT AurelienFT marked this pull request as draft January 22, 2025 14:14
@AurelienFT AurelienFT changed the base branch from master to take_vec_changes_rocksdb January 23, 2025 14:03
@AurelienFT
Copy link
Contributor Author

@xgreenx I think this fails because I should use the commit_changes for the Modifiable trait implementation that use commit_changes_with_height_update which set metadata for the new height.

So I think #2619 is not enough and we should also allow list of changes in Modifiable trait. Which means iterating to get new heights in commit_changes_with_height_update will have to run over the list changes, is it something that sounds ok to you ?

@xgreenx
Copy link
Collaborator

xgreenx commented Jan 23, 2025

We don't need to change the Modifiable trait. Only block importer needs functionality with multiple Changes. We just need to have e method for block importer that allows to commit multiple changes. This function should use commit_changes_with_height_update, where commit_changes_with_height_update knows how to work with list of changes

@AurelienFT AurelienFT marked this pull request as ready for review January 24, 2025 13:51
@AurelienFT AurelienFT marked this pull request as draft January 24, 2025 17:24
@AurelienFT AurelienFT marked this pull request as ready for review January 27, 2025 01:17
@AurelienFT AurelienFT requested a review from a team January 27, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants