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

EN-7479: storer 2 elastic bug fixes #2249

Merged
merged 5 commits into from
Aug 28, 2020
Merged

EN-7479: storer 2 elastic bug fixes #2249

merged 5 commits into from
Aug 28, 2020

Conversation

bogdan-rosianu
Copy link
Contributor

@bogdan-rosianu bogdan-rosianu commented Aug 27, 2020 •
edited

Fixed the following bugs:

  • rating was not indexed for genesis -> solved by fetching initial rating data from the nodesSetup file (if starting from genesis only)
  • return with error when a receipts' miniblock is not found
  • use round duration in format of seconds instead of milliseconds in tps benchmark updater (TPS and round time were not calculated correctly because of this difference)
  • index validators in epoch 0 (if starting from genesis only)
  • use GetMiniBlockHeadersHashes for fetching all the miniblock hashes of a header, because in the previous version, not all the miniblocks were included
  • process and index all headers for a shard which are notarized in a meta block. before, only one header per shard was indexed and this raised inconsistencies as there were missing headers.
  • used index hashed nodes coordinator with rating as the base node coordinator wouldn't have returned the same data regarding validators in an epoch or in a round.

Testing procedure:

  1. run a testnet (with this branch or at least with development branch) with elastic search on for some epochs (but limited to maximum 10k transactions/blocks/miniblocks so our comparison tool will work properly)
  2. on a machine with this branch on, inside elrond-go/cmd/storer2elastic create db/chainID directory and merge dabatases from nodes in each shard (one node per shard). For a 2 shards + meta configuration, the directory should look like:
--db 
  --- chainID
     ----- Epoch_0
            ---- Shard_0
            ---- Shard_1
            ---- Shard_metachain
      ---- Epoch_1
            ---- Shard_0
            ---- Shard_1
            ---- Shard_metachain
     ....
  1. create a new instance of elastic search (or index paths should be changed. example, from blocks -> blocks2 -> this requires code changes)
  2. adapt config.toml from storer2elastic if needed (update elastic search credentials and URL)
  3. inside storer2elastic/config place the nodesSetup.json used when running the testnet
  4. build
  5. run (no flags are necessary, hopefully)
  6. this tool should index all the data and exit when finishing or encountering an error
  7. for basic comparison, indexes can be manually checked. for advanced/automated comparison, the tool on elrond-go-tools can be used which just needs to be set up with the elastic search instances URLs and indexes paths

@bogdan-rosianu bogdan-rosianu added the type:bug Something isn't working label Aug 27, 2020
@bogdan-rosianu bogdan-rosianu self-assigned this Aug 27, 2020
@miiu96 miiu96 self-requested a review August 28, 2020 10:53
miiu96
miiu96 reviewed Aug 28, 2020
View reviewed changes
cmd/storer2elastic/dataprocessor/ratingsProcessor.go Outdated
mapToRet := make(map[uint32][]indexer.ValidatorRatingInfo)

eligible, waiting := rp.genesisNodesConfig.InitialNodesInfo()
for shardId, nodesInShard := range eligible {
Copy link
Contributor

Choose a reason for hiding this comment

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

shardID

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

cmd/storer2elastic/dataprocessor/ratingsProcessor.go Outdated
})
}
}
for shardId, nodesInShard := range waiting {
Copy link
Contributor

Choose a reason for hiding this comment

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

shardID

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

cmd/storer2elastic/dataprocessor/ratingsProcessor.go Outdated
@@ -169,6 +184,30 @@ func (rp *ratingsProcessor) getValidatorsRatingFromLeaves(leaves map[string][]by
return validatorsRatingInfo, nil
}

func (rp *ratingsProcessor) getGenesisRating() map[uint32][]indexer.ValidatorRatingInfo {
mapToRet := make(map[uint32][]indexer.ValidatorRatingInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you can renamed this variable

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

cmd/storer2elastic/dataprocessor/dataProcessor.go
if dp.startingEpoch == 0 {
validatorsPubKeys, err := nodeCoordForShard.GetAllEligibleValidatorsPublicKeys(0)
if err != nil || len(validatorsPubKeys) == 0 {
log.Warn("cannot get all eligible validatorsPubKeys", "epoch", 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you should add here continue ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, added

@sasurobert sasurobert self-requested a review August 28, 2020 11:55
sasurobert
sasurobert requested changes Aug 28, 2020
View reviewed changes
Copy link
Contributor

@sasurobert sasurobert left a comment

Choose a reason for hiding this comment

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

add unit test for the found bugs

cmd/storer2elastic/dataprocessor/dataProcessor.go Show resolved Hide resolved
cmd/storer2elastic/dataprocessor/ratingsProcessor.go Outdated Show resolved Hide resolved
cmd/storer2elastic/dataprocessor/tpsBenchmarkUpdater.go Outdated Show resolved Hide resolved
@bogdan-rosianu
Copy link
Contributor Author

add unit test for the found bugs

unit tests are in PR #2213

LucianMincu
LucianMincu requested changes Aug 28, 2020
View reviewed changes
Copy link
Contributor

@LucianMincu LucianMincu left a comment •
edited

Choose a reason for hiding this comment

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

comment below

LucianMincu
LucianMincu approved these changes Aug 28, 2020
View reviewed changes
Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

the chain ID can be read from the nodesetup.json and then mapped right inside the db folder, without changing the name inside the db folder to chainID, right?

also see DMs and attached logs and db for testing. 🙏

@LucianMincu LucianMincu merged commit 70c8f7e into development Aug 28, 2020
@LucianMincu LucianMincu deleted the EN-7479-bug-fixes-storer2elastic branch August 28, 2020 17:05
@bogdan-rosianu bogdan-rosianu mentioned this pull request Aug 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sasurobert sasurobert sasurobert approved these changes

@LucianMincu LucianMincu LucianMincu approved these changes

@miiu96 miiu96 miiu96 approved these changes

Assignees

@bogdan-rosianu bogdan-rosianu

Labels
type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bogdan-rosianu @sasurobert @LucianMincu @miiu96