-
Notifications
You must be signed in to change notification settings - Fork 198
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.
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
added log for elastic search response body parse #2413
added log for elastic search response body parse #2413
Conversation
bogdan-rosianu
commented
Oct 28, 2020
•
edited by iulianpascalau
edited by iulianpascalau
- added an additional log for displaying the body content if the parsing fails
- made each indexer error be a not shippable one, so the indexer will try to re-index the failed data
- treated error in elasticDefaultErrorResponseHandler function
- added unit tests
- changed the indexer so it can better output errors when failing to index a header
- refactored the way receipts and smart contract results are indexed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, code will need to be changed a little bit.
… try to re-index the failed data - treated error in elasticDefaultErrorResponseHandler function - added unit tests
# Conflicts: # core/indexer/elasticClientCommon_test.go
core/indexer/processTransactions.go
Outdated
@@ -136,6 +141,16 @@ func (tdp *txDatabaseProcessor) prepareTransactionsForDatabase( | |||
return append(convertMapTxsToSlice(transactions), rewardsTxs...), alteredAddresses | |||
} | |||
|
|||
func (tdp *txDatabaseProcessor) addScrsReceiverToAlteredAccounts(alteredAddress map[string]struct{}, scrs map[string]*smartContractResult.SmartContractResult) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to inject shardCoordinator, could have used the current shard ID and a range on the provided body. Miniblocks already contain destination/sender shard ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not work as the intrashard miniblock holding smart contract results is not added in the body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done yet
- re-removed shard coordinator injection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve after self implementation and self review
@@ -22,7 +22,8 @@ type DataIndexerFactory interface { | |||
// This could be an elastic search index, a MySql database or any other external services. | |||
type Indexer interface { | |||
SetTxLogsProcessor(txLogsProc process.TransactionLogProcessorDatabase) | |||
SaveBlock(body data.BodyHandler, header data.HeaderHandler, txPool map[string]data.TransactionHandler, signersIndexes []uint64, notarizedHeadersHashes []string) | |||
SaveBlock(body data.BodyHandler, header data.HeaderHandler, txPool map[string]data.TransactionHandler, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't it be better to have and argument structure here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, added TODO
7162832
core/indexer/processTransactions.go
Outdated
receipts := groupReceipts(body, txPool) | ||
scResults := groupSmartContractResults(body, txPool) | ||
tdp.addScrsReceiverToAlteredAccounts(body, alteredAddresses, scResults, selfShardID) | ||
//we can not iterate smart contracts results directly on the miniblocks contained in the block body |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"smart contracts" -> remove plural
"miniblocks" -> typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done