remove error log when block already exist blockchain #341
Conversation
blockchain/blockchain.go
Outdated
| @@ -678,6 +681,9 @@ func (bc *blockchain) MintNewSecretBlock( | |||
| func (bc *blockchain) CommitBlock(blk *Block) error { | |||
| bc.mu.Lock() | |||
| defer bc.mu.Unlock() | |||
| if b, _ := bc.GetBlockByHash(blk.HashBlock()); b != nil { | |||
There was a problem hiding this comment.
This may not work perfectly. Between L686 and L687, if there's a context switch, the db will still throw error. I don't think we need to define another error. Instead, if we see the cause of the error is db.ErrAlreadyExist, we should just ignore it, and not print error message.
consensus/consensus.go
Outdated
| @@ -8,6 +8,7 @@ package consensus | |||
|
|
|||
| import ( | |||
| "context" | |||
| "github.com/iotexproject/iotex-core/db" | |||
There was a problem hiding this comment.
File is not goimports-ed (from goimports)
blocksync/buffer.go
Outdated
| @@ -7,6 +7,7 @@ | |||
| package blocksync | |||
|
|
|||
| import ( | |||
| "github.com/iotexproject/iotex-core/db" | |||
There was a problem hiding this comment.
File is not goimports-ed (from goimports)
consensus/scheme/rolldpos/fsm.go
Outdated
| @@ -10,6 +10,7 @@ import ( | |||
| "bytes" | |||
| "context" | |||
| "encoding/hex" | |||
| "github.com/iotexproject/iotex-core/db" | |||
There was a problem hiding this comment.
File is not goimports-ed (from goimports)
Codecov Report
@@ Coverage Diff @@
## master #341 +/- ##
==========================================
- Coverage 60.77% 60.69% -0.09%
==========================================
Files 118 118
Lines 11509 11512 +3
==========================================
- Hits 6995 6987 -8
- Misses 3532 3542 +10
- Partials 982 983 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #341 +/- ##
=========================================
Coverage ? 62.72%
=========================================
Files ? 110
Lines ? 11069
Branches ? 0
=========================================
Hits ? 6943
Misses ? 3158
Partials ? 968
Continue to review full report at Codecov.
|
|
make some changes |
blocksync/buffer.go
Outdated
| @@ -65,7 +66,7 @@ func (b *blockBuffer) Flush(blk *blockchain.Block) (bool, bCheckinResult) { | |||
| break | |||
| } | |||
| delete(b.blocks, heightToSync) | |||
| if err := commitBlock(b.bc, b.ap, blk); err != nil { | |||
| if err := commitBlock(b.bc, b.ap, blk); err != nil && err != db.ErrAlreadyExist { | |||
There was a problem hiding this comment.
if err == db.ErrAlreadyExist, we should also not say "Successfully committed block". Maybe ether skip the log or say "Already exists"
|
If block already exists, ignore the err and log info msg. |
Co-Authored-By: FishMeat <330771743@qq.com>
consensus/scheme/rolldpos/fsm.go
Outdated
| Uint64("block", pendingBlock.Height()). | ||
| Msg("error when committing a block") | ||
| if err == db.ErrAlreadyExist { | ||
| err = nil |
There was a problem hiding this comment.
ineffectual assignment to err (from ineffassign)
| @@ -66,6 +67,11 @@ func (b *blockBuffer) Flush(blk *blockchain.Block) (bool, bCheckinResult) { | |||
| } | |||
| delete(b.blocks, heightToSync) | |||
| if err := commitBlock(b.bc, b.ap, blk); err != nil { | |||
| if err == db.ErrAlreadyExist { | |||
There was a problem hiding this comment.
Sorry, I just double check , and i think the right logic here is
| if err == db.ErrAlreadyExist { | |
| if err == db.ErrAlreadyExist { | |
| l.Info().Uint64("syncHeight", heightToSync).Msg("Block already exists.") | |
| } else { | |
| l.Error().Err(err).Uint64("syncHeight", heightToSync).Msg("Failed to commit the block.") | |
| break | |
| } |
And then, we don't need L75-L81
#251