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

[Storehouse] Optimize finalized reader to use block id index #5175

Merged
merged 7 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions engine/access/rpc/backend/backend_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,12 @@ func (b *backendAccounts) GetAccountAtBlockHeight(
address flow.Address,
height uint64,
) (*flow.Account, error) {
header, err := b.headers.ByHeight(height)
blockID, err := b.headers.BlockIDByHeight(height)
if err != nil {
return nil, rpc.ConvertStorageError(err)
}

blockID := header.ID()

account, err := b.getAccountAtBlock(ctx, address, blockID, header.Height)
account, err := b.getAccountAtBlock(ctx, address, blockID, height)
if err != nil {
b.log.Debug().Err(err).Msgf("failed to get account at height: %d", height)
return nil, err
Expand Down
3 changes: 1 addition & 2 deletions engine/consensus/approvals/assignment_collector_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,10 @@ func (t *AssignmentCollectorTree) selectCollectorsForFinalizedFork(startHeight,
var fork []*assignmentCollectorVertex
for height := startHeight; height <= finalizedHeight; height++ {
iter := t.forest.GetVerticesAtLevel(height)
finalizedBlock, err := t.headers.ByHeight(height)
finalizedBlockID, err := t.headers.BlockIDByHeight(height)
if err != nil {
return nil, fmt.Errorf("could not retrieve finalized block at height %d: %w", height, err)
}
finalizedBlockID := finalizedBlock.ID()
for iter.HasNext() {
vertex := iter.NextVertex().(*assignmentCollectorVertex)
if finalizedBlockID == vertex.collector.BlockID() {
Expand Down
4 changes: 2 additions & 2 deletions engine/consensus/sealing/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,11 @@ func (c *Core) processIncorporatedResult(incRes *flow.IncorporatedResult) error
// For incorporating blocks at heights that are already finalized, we check that the incorporating block
// is on the finalized fork. Otherwise, the incorporating block is orphaned, and we can drop the result.
if incorporatedAtHeight <= c.counterLastFinalizedHeight.Value() {
finalized, err := c.headers.ByHeight(incorporatedAtHeight)
finalizedID, err := c.headers.BlockIDByHeight(incorporatedAtHeight)
if err != nil {
return fmt.Errorf("could not retrieve finalized block at height %d: %w", incorporatedAtHeight, err)
}
if finalized.ID() != incRes.IncorporatedBlockID {
if finalizedID != incRes.IncorporatedBlockID {
// it means that we got incorporated incRes for a block which doesn't extend our chain
// and should be discarded from future processing
return engine.NewOutdatedInputErrorf("won't process incorporated incRes from orphan block %s", incRes.IncorporatedBlockID)
Expand Down
12 changes: 6 additions & 6 deletions engine/execution/ingestion/loader/unexecuted_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ func (e *UnexecutedLoader) finalizedUnexecutedBlocks(ctx context.Context, finali
}

for ; lastExecuted > rootBlock.Height; lastExecuted-- {
header, err := e.getHeaderByHeight(lastExecuted)
finalizedID, err := e.getHeaderByHeight(lastExecuted)
if err != nil {
return nil, fmt.Errorf("could not get header at height: %v, %w", lastExecuted, err)
}

executed, err := e.execState.IsBlockExecuted(header.Height, header.ID())
executed, err := e.execState.IsBlockExecuted(lastExecuted, finalizedID)
if err != nil {
return nil, fmt.Errorf("could not check whether block is executed: %w", err)
}
Expand All @@ -179,12 +179,12 @@ func (e *UnexecutedLoader) finalizedUnexecutedBlocks(ctx context.Context, finali
// starting from the first unexecuted block, go through each unexecuted and finalized block
// reload its block to execution queues
for height := firstUnexecuted; height <= final.Height; height++ {
header, err := e.getHeaderByHeight(height)
finalizedID, err := e.getHeaderByHeight(height)
if err != nil {
return nil, fmt.Errorf("could not get header at height: %v, %w", height, err)
}

unexecuted = append(unexecuted, header.ID())
unexecuted = append(unexecuted, finalizedID)
}

e.log.Info().
Expand Down Expand Up @@ -231,8 +231,8 @@ func (e *UnexecutedLoader) pendingUnexecutedBlocks(ctx context.Context, finalize
// if the EN is dynamically bootstrapped, the finalized blocks at height range:
// [ sealedRoot.Height, finalizedRoot.Height - 1] can not be retrieved from
// protocol state, but only from headers
func (e *UnexecutedLoader) getHeaderByHeight(height uint64) (*flow.Header, error) {
func (e *UnexecutedLoader) getHeaderByHeight(height uint64) (flow.Identifier, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func (e *UnexecutedLoader) getHeaderByHeight(height uint64) (flow.Identifier, error) {
func (e *UnexecutedLoader) getBlockIDByHeight(height uint64) (flow.Identifier, error) {

Copy link
Contributor

Choose a reason for hiding this comment

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

same question here about not using e.headers.BlockIDByHeight(height) directly

// we don't use protocol state because for dynamic boostrapped execution node
// the last executed and sealed block is below the finalized root block
return e.headers.ByHeight(height)
return e.headers.BlockIDByHeight(height)
}
8 changes: 4 additions & 4 deletions engine/execution/ingestion/loader/unfinalized_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ func (e *UnfinalizedLoader) LoadUnexecuted(ctx context.Context) ([]flow.Identifi
// reload its block to execution queues
// loading finalized blocks
for height := lastExecuted + 1; height <= final.Height; height++ {
header, err := e.getHeaderByHeight(height)
finalizedID, err := e.getHeaderByHeight(height)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use e.headers.BlockIDByHeight directly?

if err != nil {
return nil, fmt.Errorf("could not get header at height: %v, %w", height, err)
}

unexecutedFinalized = append(unexecutedFinalized, header.ID())
unexecutedFinalized = append(unexecutedFinalized, finalizedID)
}

// loaded all pending blocks
Expand All @@ -89,8 +89,8 @@ func (e *UnfinalizedLoader) LoadUnexecuted(ctx context.Context) ([]flow.Identifi
// if the EN is dynamically bootstrapped, the finalized blocks at height range:
// [ sealedRoot.Height, finalizedRoot.Height - 1] can not be retrieved from
// protocol state, but only from headers
func (e *UnfinalizedLoader) getHeaderByHeight(height uint64) (*flow.Header, error) {
func (e *UnfinalizedLoader) getHeaderByHeight(height uint64) (flow.Identifier, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func (e *UnfinalizedLoader) getHeaderByHeight(height uint64) (flow.Identifier, error) {
func (e *UnfinalizedLoader) getBlockIDByHeight(height uint64) (flow.Identifier, error) {

// we don't use protocol state because for dynamic boostrapped execution node
// the last executed and sealed block is below the finalized root block
return e.headers.ByHeight(height)
return e.headers.BlockIDByHeight(height)
}
6 changes: 3 additions & 3 deletions engine/execution/ingestion/stop/stop_control.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package stop

Check failure on line 1 in engine/execution/ingestion/stop/stop_control.go

View workflow job for this annotation

GitHub Actions / Lint (./)

: # github.com/onflow/flow-go/engine/execution/ingestion/stop [github.com/onflow/flow-go/engine/execution/ingestion/stop.test]

import (
"errors"
Expand Down Expand Up @@ -148,7 +148,7 @@
// StopControlHeaders is an interface for fetching headers
// Its jut a small subset of storage.Headers for comments see storage.Headers
type StopControlHeaders interface {
ByHeight(height uint64) (*flow.Header, error)
BlockIDByHeight(height uint64) (flow.Identifier, error)
}

// NewStopControl creates new StopControl.
Expand Down Expand Up @@ -476,12 +476,12 @@

// Let's find the ID of the block that should be executed last
// which is the parent of the block at the stopHeight
header, err := s.headers.ByHeight(s.stopBoundary.StopBeforeHeight - 1)
finalizedID, err := s.headers.BlockIDByHeight(s.stopBoundary.StopBeforeHeight - 1)
if err != nil {
handleErr(fmt.Errorf("failed to get header by height: %w", err))
return
}
parentID = header.ID()
parentID = finalizedID
}

s.stopBoundary.stopAfterExecuting = parentID
Expand Down
4 changes: 2 additions & 2 deletions engine/execution/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,11 +487,11 @@ func (s *state) GetHighestExecutedBlockID(ctx context.Context) (uint64, flow.Ide
// when storehouse is enabled, the highest executed block is consisted as
// the highest finalized and executed block
height := s.GetHighestFinalizedExecuted()
header, err := s.headers.ByHeight(height)
finalizedID, err := s.headers.BlockIDByHeight(height)
if err != nil {
return 0, flow.ZeroID, fmt.Errorf("could not get header by height %v: %w", height, err)
}
return height, header.ID(), nil
return height, finalizedID, nil
}

var blockID flow.Identifier
Expand Down
4 changes: 2 additions & 2 deletions module/finalizedreader/finalizedreader.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ func (r *FinalizedReader) FinalizedBlockIDAtHeight(height uint64) (flow.Identifi
return flow.ZeroID, fmt.Errorf("height not finalized (%v): %w", height, storage.ErrNotFound)
}

header, err := r.headers.ByHeight(height)
finalizedID, err := r.headers.BlockIDByHeight(height)
if err != nil {
return flow.ZeroID, err
}

return header.ID(), nil
return finalizedID, nil
}

// BlockFinalized implements the protocol.Consumer interface, which allows FinalizedReader
Expand Down
4 changes: 2 additions & 2 deletions module/finalizer/consensus/finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@
}

if pending.Height <= finalized {
dup, err := f.headers.ByHeight(pending.Height)
dupID, err := f.headers.BlockIDByHeight(pending.Height)
if err != nil {
return fmt.Errorf("could not retrieve finalized equivalent: %w", err)
}
if dup.ID() != blockID {
if dupID != blockID {
return fmt.Errorf("cannot finalize pending block conflicting with finalized state (height: %d, pending: %x, finalized: %x)", pending.Height, blockID, dup.ID())

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Lint (./)

undefined: dup) (typecheck)

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Lint (./)

undefined: dup) (typecheck)

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Lint (./)

undefined: dup) (typecheck)

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/verification)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/verification)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/verification)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/verification)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/verification)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/collection)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/collection)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/collection)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/collection)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/collection)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/execution)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/execution)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/execution)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/execution)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/execution)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (others)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (others)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (others)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (others)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (others)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (cmd)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (cmd)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (cmd)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (cmd)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (cmd)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (consensus)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (consensus)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (consensus)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (consensus)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (consensus)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/common)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/common)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/common)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/common)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine/common)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine)

undefined: dup

Check failure on line 83 in module/finalizer/consensus/finalizer.go

View workflow job for this annotation

GitHub Actions / Unit Tests (engine)

undefined: dup
}
return nil
}
Expand Down
Loading