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.

Sign up for GitHub

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

Jump to bottom

Integrate indexer to work with partial executed mini-blocks #4052

Merged
merged 10 commits into from
Jun 7, 2022
Merged

Integrate indexer to work with partial executed mini-blocks #4052

merged 10 commits into from
Jun 7, 2022

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented May 3, 2022 •
edited

  • Updated elastic indexer in order to index correctly partial mini-blocks
  • Fix the API block in order to return only executed transactions in case of partial-execution

Base automatically changed from feat/partial-mb-execution to development May 24, 2022 17:46
@iulianpascalau iulianpascalau self-requested a review May 31, 2022 12:59
iulianpascalau
iulianpascalau reviewed May 31, 2022
View reviewed changes
node/external/blockAPI/baseBlock.go
@@ -106,13 +106,19 @@ func (bap *baseAPIBlockProcessor) prepareAPIMiniblock(miniblock *block.MiniBlock
DestinationShard: miniblock.ReceiverShardID,
}
if withTxs {
bap.getAndAttachTxsToMbByEpoch(mbHash, miniblock, epoch, miniblockAPI)
firstProcessed := int32(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is correct for the intrashard miniblock, let's rename prepareAPIMiniblock to something like prepareAPIMiniblockForIntrashard and extractMbsFromBatch to something like extractIntrashardMbsFromBatch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

node/external/blockAPI/baseBlock.go Outdated

func extractExecutedTxHashes(mbTxHashes [][]byte, firstProcessed, lastProcessed int32) [][]byte {
executedTxHashes := make([][]byte, 0)
for txIndex, txHash := range mbTxHashes {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a little bit of CPU processing time. A slightly optimized variant:

invalidIndexes := firstProcessed < 0 || lastProcessed >= len(mbTxHashes) || firstProcessed > lastProcessed
if invalidIndexes {
    log.Warn("extractExecutedTxHashes encountered invalid indexes", "firstProcessed", firstProcessed, 
"lastProcessed", lastProcessed, "len(mbTxHashes)", len(mbTxHashes))
    return executedTxHashes
}

for txIndex := executedTxHashes; txIndex <= lastProcessed; txIndex++{
    executedTxHashes = append(executedTxHashes, mbTxHashes[txIndex])
} 

return executedTxHashes

+add edgecases tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bogdan-rosianu
bogdan-rosianu reviewed May 31, 2022
View reviewed changes
go.mod Outdated
@@ -8,7 +8,7 @@ require (
github.com/ElrondNetwork/arwen-wasm-vm/v1_4 v1.4.51
github.com/ElrondNetwork/concurrent-map v0.1.3
github.com/ElrondNetwork/covalent-indexer-go v1.0.6
github.com/ElrondNetwork/elastic-indexer-go v1.2.25
github.com/ElrondNetwork/elastic-indexer-go v1.2.29-0.20220530074622-4a44bb8e4c7b
Copy link
Contributor

Choose a reason for hiding this comment

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

proper release before merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added proper release

node/external/blockAPI/baseBlock.go Outdated
func extractExecutedTxHashes(mbTxHashes [][]byte, firstProcessed, lastProcessed int32) [][]byte {
invalidIndexes := firstProcessed < 0 || lastProcessed >= int32(len(mbTxHashes)) || firstProcessed > lastProcessed
if invalidIndexes {
log.Warn("extractExecutedTxHashes encountered invalid indexes", "firstProcessed", firstProcessed,
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: indices instead of indexes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

node/external/blockAPI/baseBlock_test.go Outdated
res = extractExecutedTxHashes(array, 0, int32(len(array))+1)
require.Equal(t, res, array)

res = extractExecutedTxHashes(array, 0, int32(len(array))+1)
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated check: res = extractExecutedTxHashes(array, 0, int32(len(array))+1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2022 •
edited

Codecov Report

Merging #4052 (2b12b45) into development (4c6761d) will increase coverage by 0.58%.
The diff coverage is 91.87%.

❗ Current head 2b12b45 differs from pull request most recent head b723afd. Consider uploading reports for the commit b723afd to get more accurate results

@@               Coverage Diff               @@
##           development    #4052      +/-   ##
===============================================
+ Coverage        75.25%   75.83%   +0.58%     
===============================================
  Files              614      644      +30     
  Lines            82115    85082    +2967     
===============================================
+ Hits             61794    64523    +2729     
- Misses           15643    15773     +130     
- Partials          4678     4786     +108
Impacted Files Coverage Δ
dataRetriever/interface.go 12.12% <ø> (ø)
epochStart/bootstrap/baseStorageHandler.go 61.19% <ø> (ø)
factory/bootstrapComponentsHandler.go 61.40% <0.00%> (ø)
factory/consensusComponentsHandler.go 60.56% <0.00%> (+4.71%) ⬆️
factory/cryptoComponentsHandler.go 66.21% <0.00%> (ø)
factory/dataComponentsHandler.go 71.08% <0.00%> (ø)
factory/heartbeatComponentsHandler.go 64.17% <0.00%> (ø)
factory/networkComponentsHandler.go 78.49% <0.00%> (ø)
factory/stateComponentsHandler.go 72.04% <0.00%> (ø)
factory/statusComponentsHandler.go 16.45% <0.00%> (ø)
... and 151 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6268d14...b723afd. Read the comment docs.

popenta
popenta approved these changes Jun 7, 2022
View reviewed changes
Copy link
Contributor

@popenta popenta left a comment

Choose a reason for hiding this comment

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

@@ Log scanner @@

integrate-indexer-partion-exec

================================================================================

  • Known Warnings 22
  • New Warnings 8
  • Known Errors 0
  • New Errors 2
  • Panics 3
    ================================================================================

@miiu96 miiu96 merged commit c858849 into development Jun 7, 2022
@miiu96 miiu96 deleted the integrate-indexer-partion-exec branch June 7, 2022 08:00
@miiu96 miiu96 self-assigned this Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iulianpascalau iulianpascalau iulianpascalau approved these changes

@bogdan-rosianu bogdan-rosianu bogdan-rosianu approved these changes

@popenta popenta popenta approved these changes

Assignees

@miiu96 miiu96

Labels
area:indexer impact:low size:small type:feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@miiu96 @codecov-commenter @iulianpascalau @bogdan-rosianu @popenta