Improve and test getProcessingAncestor #2956
Conversation
StephenButtolph
added
consensus
| @@ -38,104 +38,106 @@ type metrics struct { | |||
| issued *prometheus.CounterVec | |||
| } | |||
|
|
|||
| func (m *metrics) Initialize(namespace string, reg prometheus.Registerer) error { | |||
| func newMetrics(namespace string, reg prometheus.Registerer) (*metrics, error) { | |||
There was a problem hiding this comment.
This change allows us to create the metrics in a non-jank way.
| // It's possible that the blocks we just added to consensus were decided | ||
| // immediately by votes that were pending their issuance. If this is the | ||
| // case, we should not be requesting any chits. |
There was a problem hiding this comment.
This is a (very minor) bug fix.
| if t.Consensus.Processing(bubbledVote) { | ||
| t.Ctx.Log.Verbo("applying vote", | ||
| zap.Stringer("initialVoteID", initialVote), | ||
| zap.Stringer("bubbledVoteID", bubbledVote), | ||
| ) | ||
| if bubbledVote != initialVote { | ||
| t.numProcessingAncestorFetchesSucceeded.Inc() | ||
| } else { | ||
| t.numProcessingAncestorFetchesUnneeded.Inc() | ||
| } | ||
| return bubbledVote, true | ||
| } |
There was a problem hiding this comment.
Moving the Processing check before getBlock avoids calling VM.GetBlock when getProcessingAncestor is called with a processing block.
| vm.GetBlockF = func(_ context.Context, blkID ids.ID) (snowman.Block, error) { | ||
| switch blkID { | ||
| case GenesisID: | ||
| return Genesis, nil | ||
| case issuedBlk.ID(): | ||
| return issuedBlk, nil | ||
| case missingBlk.ID(): | ||
| return missingBlk, nil | ||
| case blockingBlk.ID(): | ||
| return blockingBlk, nil | ||
| default: | ||
| return nil, errUnknownBlock | ||
| } | ||
| } |
There was a problem hiding this comment.
This test was previously pretty broken because the engine assumes that blocks that have been Verifyed are GetBlockable. So once we issueed missingBlk, the engine was in a weird state.
| // processing in consensus. If no ancestor could be found, false is returned. | ||
| // | ||
| // Note: If [initialVote] is processing, then [initialVote] will be returned. | ||
| func (t *Transitive) getProcessingAncestor(ctx context.Context, initialVote ids.ID) (ids.ID, bool) { |
There was a problem hiding this comment.
This was moved into this file because all of the fields that are used on are this struct. I was considering moving this function off the structs entirely and just passing in the fields... But there are quite a few things that can be accessed... Especially because of the getBlock call.
Why this should be merged
GetBlockduring the normal case of bubbling votes (when the voted item is provided).getProcessingAncestorbehavior.How this works
Just moves code around slightly.
How this was tested