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

[blockindex] simplify indexer height checking #4466

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
16 changes: 1 addition & 15 deletions blockchain/blockdao/blockindexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ type (
DeleteTipBlock(context.Context, *block.Block) error
}

// BlockIndexerWithStart defines an interface to accept block to build index from a start height
BlockIndexerWithStart interface {
BlockIndexer
// StartHeight returns the start height of the indexer
StartHeight() uint64
}

// BlockIndexerChecker defines a checker of block indexer
BlockIndexerChecker struct {
dao BlockDAO
Expand Down Expand Up @@ -74,14 +67,7 @@ func (bic *BlockIndexerChecker) CheckIndexer(ctx context.Context, indexer BlockI
if targetHeight == 0 || targetHeight > daoTip {
targetHeight = daoTip
}
startHeight := tipHeight + 1
if indexerWS, ok := indexer.(BlockIndexerWithStart); ok {
indexStartHeight := indexerWS.StartHeight()
if indexStartHeight > startHeight {
startHeight = indexStartHeight
}
}
for i := startHeight; i <= targetHeight; i++ {
for i := tipHeight + 1; i <= targetHeight; i++ {
Copy link
Member Author

@dustinxie dustinxie Oct 29, 2024

Choose a reason for hiding this comment

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

currently tipHeight at L52 returns 0, so we need extra code (L78-84) to check every indexer's start height, which is somewhat confusing, since the same code (to check every indexer's start height) also appears in SyncIndexer. So that should be handled by SyncIndexer, and we only need to get it's height (at L52)

// ternimate if context is done
if err := ctx.Err(); err != nil {
return errors.Wrap(err, "terminate the indexer checking")
Expand Down
11 changes: 5 additions & 6 deletions blockindex/contractstaking/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,11 @@ func (s *Indexer) Stop(ctx context.Context) error {

// Height returns the tip block height
func (s *Indexer) Height() (uint64, error) {
return s.cache.Height(), nil
}

// StartHeight returns the start height of the indexer
func (s *Indexer) StartHeight() uint64 {
return s.config.ContractDeployHeight
h := s.cache.Height()
if h < s.config.ContractDeployHeight {
h = s.config.ContractDeployHeight
}
return h, nil
Copy link
Member Author

@dustinxie dustinxie Oct 29, 2024

Choose a reason for hiding this comment

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

combine the concept of start height into Height(). It is used by PutBlock() to check if the incoming block should be put/handled, so simply return the max of height and startHeight -- if the incoming block's height is smaller, nothing needs to be done

}

// ContractAddress returns the contract address
Expand Down
1 change: 0 additions & 1 deletion blockindex/contractstaking/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func TestContractStakingIndexerLoadCache(t *testing.T) {
newHeight, err := newIndexer.Height()
r.NoError(err)
r.Equal(height, newHeight)
r.Equal(startHeight, newIndexer.StartHeight())
tbc, err = newIndexer.TotalBucketCount(height)
r.EqualValues(1, tbc)
r.NoError(err)
Expand Down
42 changes: 3 additions & 39 deletions blockindex/sync_indexers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import (
// SyncIndexers is a special index that includes multiple indexes,
// which stay in sync when blocks are added.
type SyncIndexers struct {
indexers []blockdao.BlockIndexer
startHeights []uint64 // start height of each indexer, which will be determined when the indexer is started
minStartHeight uint64 // minimum start height of all indexers
indexers []blockdao.BlockIndexer
}

// NewSyncIndexers creates a new SyncIndexers
Expand All @@ -33,7 +31,7 @@ func (ig *SyncIndexers) Start(ctx context.Context) error {
return err
}
}
return ig.initStartHeight()
return nil
}

// Stop stops the indexer group
Expand All @@ -48,11 +46,7 @@ func (ig *SyncIndexers) Stop(ctx context.Context) error {

// PutBlock puts a block into the indexers in the group
func (ig *SyncIndexers) PutBlock(ctx context.Context, blk *block.Block) error {
for i, indexer := range ig.indexers {
// check if the block is higher than the indexer's start height
if blk.Height() < ig.startHeights[i] {
continue
}
for _, indexer := range ig.indexers {
// check if the block is higher than the indexer's height
height, err := indexer.Height()
if err != nil {
Expand All @@ -79,11 +73,6 @@ func (ig *SyncIndexers) DeleteTipBlock(ctx context.Context, blk *block.Block) er
return nil
}

// StartHeight returns the minimum start height of the indexers in the group
func (ig *SyncIndexers) StartHeight() uint64 {
return ig.minStartHeight
}

// Height returns the minimum height of the indexers in the group
func (ig *SyncIndexers) Height() (uint64, error) {
var height uint64
Expand All @@ -98,28 +87,3 @@ func (ig *SyncIndexers) Height() (uint64, error) {
}
return height, nil
}

// initStartHeight initializes the start height of the indexers in the group
// for every indexer, the start height is the maximum of tipheight+1 and startheight
func (ig *SyncIndexers) initStartHeight() error {
ig.minStartHeight = 0
ig.startHeights = make([]uint64, len(ig.indexers))
for i, indexer := range ig.indexers {
tipHeight, err := indexer.Height()
if err != nil {
return err
}
indexStartHeight := tipHeight + 1
if indexerWithStart, ok := indexer.(blockdao.BlockIndexerWithStart); ok {
startHeight := indexerWithStart.StartHeight()
if startHeight > indexStartHeight {
indexStartHeight = startHeight
}
}
ig.startHeights[i] = indexStartHeight
if i == 0 || indexStartHeight < ig.minStartHeight {
ig.minStartHeight = indexStartHeight
}
}
return nil
}
2 changes: 0 additions & 2 deletions blockindex/sync_indexers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ func TestSyncIndexers_StartHeight(t *testing.T) {
ig := NewSyncIndexers(indexers...)
err := ig.Start(context.Background())
require.NoError(err)
height := ig.StartHeight()
require.Equal(c.expect, height)
})
}

Expand Down
3 changes: 3 additions & 0 deletions systemcontractindex/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ func (s *IndexerCommon) ContractAddress() string { return s.contractAddress }

// Height returns the tip block height
func (s *IndexerCommon) Height() uint64 {
if s.height < s.startHeight {
return s.startHeight
}
return s.height
}

Expand Down
8 changes: 0 additions & 8 deletions systemcontractindex/stakingindex/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type (
StakingIndexer interface {
lifecycle.StartStopper
Height() (uint64, error)
StartHeight() uint64
ContractAddress() string
Buckets(height uint64) ([]*VoteBucket, error)
Bucket(id uint64, height uint64) (*VoteBucket, bool, error)
Expand Down Expand Up @@ -90,13 +89,6 @@ func (s *Indexer) Height() (uint64, error) {
return s.common.Height(), nil
}

// StartHeight returns the start height of the indexer
func (s *Indexer) StartHeight() uint64 {
s.mutex.RLock()
defer s.mutex.RUnlock()
return s.common.StartHeight()
}

// ContractAddress returns the contract address
func (s *Indexer) ContractAddress() string {
s.mutex.RLock()
Expand Down
Loading