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

PostBlock optimization #1972

Merged
merged 11 commits into from
Oct 16, 2023
Merged

PostBlock optimization #1972

merged 11 commits into from
Oct 16, 2023

Conversation

goran-ethernal
Copy link
Collaborator

@goran-ethernal goran-ethernal commented Oct 9, 2023

Description

This PR optimizes PostBlock and OnBlockInserted handlers on consensus_runtime and its components (state_sync_manager, checkpoint_manager, stake_manager and proposer_calculator).

Before, each of the given components was iterating through the receipts of given finalized block, in OnBlockInserted function, to get the required events (for example, checkpoint_manager was getting Exit events, stake_manager was getting Transfer events, etc). To optimize this behavior, and remove iterating through the same receipts multiple times, this PR introduces these concepts:

EventProvider

EventProvider is a component which retrieves receipts of a range of blocks or a single block to get desired events from them.
It requires components to Subscribe to it, using the Subscribe(subscriber EventSubscriber) function.
Each of components that subscribe to the EventProvider need to implement the EventSubscriber interface:

// EventSubscriber specifies functions needed for a component to subscribe to eventProvider
type EventSubscriber interface {
	GetLogFilters() map[types.Address][]types.Hash
	AddLog(header *types.Header, log *ethgo.Log, dbTx DBTransaction) error
}

GetLogFilters function returns the map, where key is the address of a contract on which we want to get some type of events, and value is the signature or ID of the event on that contract that the subscriber needs. This map is used to filter out events from receipts so that it can return correct results to the given subscriber.

AddLog function is the handler function on the subscriber. When the EventProvider encounters an event that matches one of the filters in the GetLogFilters function for that subscriber, it will call the AddLog function on the subscriber, so that he can handle the given event (store it in memory, to a db, etc).

This way, we provide only one iteration through the blocks and receipts to get the desired events of different components, plus, if AddLog returns an error on any component, EventProvider immediately stops going through receipts, meaning that state of one component is tied to the state of other components, keeping the state more atomic, where either all components get updated, or no component does. This helps the consensus to have a more cleaner state. If one component is more advanced than the other, it may cause serious issues to running a more correct consensus.

Global DB Transaction in OnBlockInserted

In state.go we added a func (s *State) beginTx(isWriteTx bool) function so that it can open a manually maintained transaction. This is needed for keeping the correct state of each component as mentioned in the previous section. This means that OnBlockInserted function has more control over its components. If either of them fail to save their events, or to do PostBlock functions, global db transaction is rolledback, and state of all components for that block is cleaned and thrown away, keeping the consensus more accurate, and error prone.

At the beginning of the OnBlockInserted function in conesnsus_runtime, a global db transaction is opened, and only after all components insert their events (gotten from EventProvider) and do PostBlock succesfuly, only then we will commit the transaction and save the components state to db. If any of them fail in these actions, transaction is rolled back.

Exceptions to previously mentioned concepts

PostBlock function on checkpoint_manager is done after committing the global db transaction, since it does not change any state in boltDB, it only sends a checkpoint in a separate thread.

validator_snapshot.go - GetSnapshot function will open a db transaction if it is not passed to it. The reason behind this is that, GetSnapshot function is called from multiple go routines: OnBlockInserted, starting a new sequence in polybft, validating blocks in fsm, etc., and what can happen is that one routine can take its lock, but in the mean time, OnBlockInserted opens a global db transaction, which can result in a deadlock, since in OnBlockInserted proposer calculator, updates the proposer snapshot, by calling this GetSnapshot function. So the first routine gets the lock on validatorSnapshot, but it can not do anything with the db since OnBlockInserted holds the global db transaction. And OnBlockInserted can not update the proposer snapshot, because other routine holds the validatorSnapshot lock.
This is mitigated by opening db transaction on validatorSnapshot if it is not already passed.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

@goran-ethernal goran-ethernal self-assigned this Oct 9, 2023
@goran-ethernal goran-ethernal force-pushed the post-block-optimization branch 5 times, most recently from 4b3c0dd to 855900a Compare October 10, 2023 13:42
@goran-ethernal goran-ethernal requested a review from a team October 10, 2023 14:11
@goran-ethernal goran-ethernal marked this pull request as ready for review October 10, 2023 14:13
consensus/polybft/state_event_getter.go Outdated Show resolved Hide resolved
consensus/polybft/state.go Outdated Show resolved Hide resolved
consensus/polybft/state_event_getter.go Outdated Show resolved Hide resolved
consensus/polybft/state_event_getter.go Outdated Show resolved Hide resolved
consensus/polybft/state_event_getter.go Show resolved Hide resolved
consensus/polybft/state_event_getter.go Show resolved Hide resolved
consensus/polybft/state_event_getter.go Show resolved Hide resolved
consensus/polybft/state.go Outdated Show resolved Hide resolved
consensus/polybft/stake_manager.go Show resolved Hide resolved
@goran-ethernal goran-ethernal merged commit 820e21a into develop Oct 16, 2023
10 checks passed
@goran-ethernal goran-ethernal deleted the post-block-optimization branch October 16, 2023 13:36
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants