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.

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

Bug fixes elastic search indexer #1997

Merged
merged 4 commits into from Jun 24, 2020
Merged

Bug fixes elastic search indexer #1997

merged 4 commits into from Jun 24, 2020

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Jun 23, 2020

No description provided.

@sasurobert sasurobert self-requested a review June 23, 2020 09:01
if err != nil {
log.Warn("indexer: response body",
"body", string(responseBody))
log.Warn("indexer:cannot read from response body", "error", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

return nil, err here

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.

@@ -104,16 +104,16 @@ func (ei *elasticIndexer) SaveBlock(
}

txsSizeInBytes := computeSizeOfTxs(ei.marshalizer, txPool)
go ei.database.SaveHeader(headerHandler, signersIndexes, body, notarizedHeadersHashes, txsSizeInBytes)
ei.database.SaveHeader(headerHandler, signersIndexes, body, notarizedHeadersHashes, txsSizeInBytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

may be leave to go routines - add a go routine throttler ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the moment i will let as it is because method SaveBlock is called on a goroutine and i want the writing of header, miniblocks and transaction to be serial because this write do a lot of requests to elasticsearch database because have to check if miniblocks and transactions exits in database.

@miiu96 miiu96 changed the title Bug fix elastic search indexer Bug fixes elastic search indexer Jun 23, 2020
var responseBody []byte
responseBody, err = ioutil.ReadAll(res.Body)
if err != nil {
log.Warn("indexer: response body",
Copy link
Contributor

Choose a reason for hiding this comment

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

You should delete this Warn, as responeBody should be nil if err != nil. Did you want to log res.Body? If you want to keep this warn you can add it in the next warn as additional info:
log.Warn("indexer:cannot read from response body", "body", string(responseBody), "error", err)

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.

var decodedBody object
if err := json.NewDecoder(res.Body).Decode(&decodedBody); err != nil {
if err := json.Unmarshal(responseBody, &decodedBody); err != nil {
log.Warn("indexer: response body",
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this warn and add it in the next Warn as additional info, like suggested above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

body

txsPool[hashes[i]] = &txs[i]
}
for i := 0; i < 1000; i++ {
numTransactions := 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Move these two lines outside the for

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.

}
header := &dataBlock.Header{}
txsPool := make(map[string]data.TransactionHandler)
for i := 0; i < numTransactions; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use j instead i. The outer for uses also i for iteration

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.

TxHashes: make([][]byte, numTransactions),
Type: dataBlock.TxBlock,
}
for i := 0; i < numTransactions; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use j instead i. The outer for uses also i for iteration

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.

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.

System tests passed.

@LucianMincu LucianMincu merged commit 28c90b7 into release-candidate Jun 24, 2020
@LucianMincu LucianMincu deleted the bug-fix-es branch June 24, 2020 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants