Improve requesting of missing mini blocks dest me #5176
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## rc/v1.6.0 #5176 +/- ##
==========================================
Coverage 80.12% 80.13%
==========================================
Files 708 708
Lines 93880 93910 +30
==========================================
+ Hits 75223 75251 +28
- Misses 13318 13320 +2
Partials 5339 5339
☔ View full report in Codecov by Sentry. |
…niblocks-dest-me' into improve-requesting-of-missing-miniblocks-dest-me
… accepted as a whitelisted mini blocks for the transaction requester
process/coordinator/process.go
Outdated
| @@ -748,6 +749,29 @@ func (tc *transactionCoordinator) CreateMbsAndProcessCrossShardTransactionsDstMe | |||
| return createMBDestMeExecutionInfo.miniBlocks, createMBDestMeExecutionInfo.numTxAdded, allMBsProcessed, nil | |||
| } | |||
|
|
|||
| func (tc *transactionCoordinator) requestMissingMiniBlocks(mbsInfo []*data.MiniBlockInfo) { | |||
There was a problem hiding this comment.
one other problem is to stop requesting transactions after the first miniblock where there are requestedTxs (line 689):
if requestedTxs > 0 { shouldSkipShard[miniBlockInfo.SenderShardID] = true
The requests could be done for all the miniblocks that could be processed in a round.
There was a problem hiding this comment.
Yes, but this is solved with this new approach by calling requestMissingMiniBlocks above. For all these mbs requested in this method, once they will be received in L1138 -> method func (tc *transactionCoordinator) receivedMiniBlock(key []byte, value interface{}) we will finally know all the tx hashes and request them if missing in L1162 -> numTxsRequested := preproc.RequestTransactionsForMiniBlock(miniBlock). Actually you cannot request txs for the missing mbs as you don't know their hashes. But I can add the txs request for all the miniblocks which are not missing in the same method -> requestMissingMiniBlocks.
process/coordinator/process.go
Outdated
| @@ -748,6 +749,29 @@ func (tc *transactionCoordinator) CreateMbsAndProcessCrossShardTransactionsDstMe | |||
| return createMBDestMeExecutionInfo.miniBlocks, createMBDestMeExecutionInfo.numTxAdded, allMBsProcessed, nil | |||
| } | |||
|
|
|||
| func (tc *transactionCoordinator) requestMissingMiniBlocks(mbsInfo []*data.MiniBlockInfo) { | |||
There was a problem hiding this comment.
functionality of requestMissingMiniBlocks and RequestMiniBlocks is very similar, could the refactored with a method that gets a list of miniblockHashes and does the request for those and be called from both other methods.
There was a problem hiding this comment.
Right, will do this refactor
…niblocks-dest-me' into improve-requesting-of-missing-miniblocks-dest-me
| continue | ||
| } | ||
|
|
||
| numTxsRequested := preproc.RequestTransactionsForMiniBlock(miniBlock) |
There was a problem hiding this comment.
this will cause the requesting of the missing transactions to be done before the miniblocks are actually requested. This will work as the request handler will whitelist the required data and the interceptors will then accept the missing txs. We should prevent the future disruption of the whitelist mechanisms between the request handler and the interceptors.
There was a problem hiding this comment.
Actually this is the case when some of the mini blocks, from the same header, are already in pool and it requests txs from them, if they are missing.
iulianpascalau
deleted the
improve-requesting-of-missing-miniblocks-dest-me
branch
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?