[blockdao] introduce blockindexer with start height #3869
Conversation
envestcc
requested review from
CoderZhi,
dustinxie,
Liuhaai and
a team
as code owners
Codecov Report
@@ Coverage Diff @@
## master #3869 +/- ##
==========================================
+ Coverage 75.38% 75.79% +0.41%
==========================================
Files 303 318 +15
Lines 25923 27150 +1227
==========================================
+ Hits 19541 20578 +1037
- Misses 5360 5517 +157
- Partials 1022 1055 +33
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| // PutBlock puts a block into the block indexer group | ||
| // for every indexer, if StartHeight <= block.Height && Height < block.Height, put the block | ||
| func (g *BlockIndexerWithStartGroup) PutBlock(ctx context.Context, blk *block.Block) error { | ||
| for _, indexer := range g.indexers { |
There was a problem hiding this comment.
could the height check logic be put inside each indexer?
There was a problem hiding this comment.
For an indexer, the difference between adding to BlockDao and adding to BlockIndexerWithStartGroup is as follows:
- When adding to
BlockDao, blocks with heights greater thanHeight()will enterPutBlock(). - When adding to
BlockIndexerWithStartGroup, only blocks with heights greater thanStartHeight()and greater thanHeight()will enterPutBlock().
Hence, it is preferable to place this logic here instead of duplicating it in each indexer.
chainservice/builder.go
Outdated
| @@ -261,6 +261,9 @@ func (builder *Builder) buildBlockDAO(forTest bool) error { | |||
| if builder.cs.contractStakingIndexer != nil { | |||
| indexers = append(indexers, builder.cs.contractStakingIndexer) | |||
| } | |||
| startGroupIndexer := blockdao.NewBlockIndexerWithStartGroup() | |||
There was a problem hiding this comment.
will be addressed in the subsequent pr
| // BlockIndexerWithStartGroup defines a group of block indexers with start height | ||
| BlockIndexerWithStartGroup struct { | ||
| indexers []BlockIndexerWithStart | ||
| tipHeightFunc func() (uint64, error) |
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
i think we don't need to change Start(), but need to change dao.checkIndexer
inside there, if it's a IndexerWithStart, then we only do the check indexer for blocks that's > each indexer's StartHeight
| // first, for every indexer, use max of Height and StartHeight | ||
| // then, use the min of all indexers | ||
| // finally, use the min of the max and maxValidHeight | ||
| func (g *BlockIndexerWithStartGroup) Height() (uint64, error) { |
There was a problem hiding this comment.
why we need a common Height() for this indexer group?
chainservice/builder.go
Outdated
| @@ -261,6 +261,7 @@ func (builder *Builder) buildBlockDAO(forTest bool) error { | |||
| if builder.cs.contractStakingIndexer != nil { | |||
| indexers = append(indexers, builder.cs.contractStakingIndexer) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
revert, so no need to change this file
envestcc
force-pushed
the
indexer_with_start
branch
from
5dffdc1
to
bf4a755
Compare
|
SonarCloud Quality Gate failed.
|
Description
To optimize the startup check of the those indexer who don't need lower blocks, introducing a new feature in CheckIndexer.
If indexer implemented
StartHeight()method,PutBlockwill start at maximum betweenStartHeightwithHeightwhen indexer checking at startupFixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: