Change logs from map to slice #3655

bogdan-rosianu · 2021-12-21T14:20:45Z

Change logs and events from map to slice -> in order to ensure the order of the events when saving them into outport drivers

codecov · 2021-12-21T14:30:42Z

Codecov Report

Merging #3655 (046d0e2) into development (9899a97) will increase coverage by 0.00%.
The diff coverage is 89.36%.

@@             Coverage Diff              @@
##           development    #3655   +/-   ##
============================================
  Coverage        73.68%   73.69%           
============================================
  Files              587      587           
  Lines            76031    76040    +9     
============================================
+ Hits             56027    56035    +8     
- Misses           15561    15563    +2     
+ Partials          4443     4442    -1

Impacted Files	Coverage Δ
process/transactionLog/process.go	`86.02% <77.27%> (-2.22%)`	⬇️
dblookupext/esdtSupply/esdtSuppliesProcessor.go	`73.52% <100.00%> (+3.52%)`	⬆️
dblookupext/esdtSupply/logsGetter.go	`77.77% <100.00%> (-1.39%)`	⬇️
dblookupext/esdtSupply/logsProcessor.go	`72.80% <100.00%> (ø)`
dblookupext/historyRepository.go	`76.09% <100.00%> (ø)`
process/coordinator/process.go	`76.60% <100.00%> (ø)`
process/transactionLog/printTxLogProcessor.go	`100.00% <100.00%> (ø)`
p2p/libp2p/netMessenger.go	`75.00% <0.00%> (ø)`
process/block/baseProcess.go	`67.77% <0.00%> (+0.24%)`	⬆️

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 957a2ff...046d0e2. Read the comment docs.

dblookupext/esdtSupply/logsGetter.go

iulianpascalau · 2021-12-21T16:08:02Z

go.mod

-	github.com/ElrondNetwork/covalent-indexer-go v1.0.4
-	github.com/ElrondNetwork/elastic-indexer-go v1.1.28
-	github.com/ElrondNetwork/elrond-go-core v1.1.5
+	github.com/ElrondNetwork/covalent-indexer-go v1.0.5-0.20211221133537-5f9e7df77444


proper relases

iulianpascalau · 2021-12-21T16:12:54Z

process/transactionLog/process.go

@@ -53,7 +55,8 @@ func NewTxLogProcessor(args ArgTxLogProcessor) (*txLogProcessor, error) {
 	return &txLogProcessor{
 		storer:      storer,
 		marshalizer: args.Marshalizer,
-		logs:        make(map[string]data.LogHandler),
+		logs:        make([]indexer.LogData, 0),
+		logsIndices: make(map[string]int),


Strongly suggest to use slices of pointers for the logs, map[string]*indexer.LogData for the logsMap on L59
reasons:

for...range on a slice of structs does not behave as expected, can end up with hard to catch bugs

we are avoiding copying the same data over and over again. Using pointers we will copy only a uint64 value.

process/transactionLog/process.go

…hange-logs-map-to-slice

dblookupext/historyRepository.go

ssd04 · 2021-12-22T13:18:39Z

dblookupext/interface.go

-		logs map[string]data.LogHandler,
-	) error
-
+	RecordBlock(blockHeaderHash []byte, blockHeader data.HeaderHandler, blockBody data.BodyHandler, scrResultsFromPool map[string]data.TransactionHandler, receiptsFromPool map[string]data.TransactionHandler, logs []*data.LogData, ) error


same here with the additional ,

done, thanks

# Conflicts: # go.mod # go.sum

ssd04 · 2021-12-22T15:01:49Z

process/transactionLog/process.go

-		logsMap[key] = value
-	}
+	logsSlice := make([]*data.LogData, 0, len(tlp.logs))
+	logsSlice = append(logsSlice, tlp.logs...)


is there a particular reason to not just set logsSlice := tlp.logs ?

yes, it's safer to make a clone of the slice

iulianpascalau · 2021-12-23T12:04:44Z

dblookupext/esdtSupply/esdtSuppliesProcessor.go

-	return sp.logsProc.processLogs(blockNonce, logs, false)
+	logsMap := make(map[string]*data.LogData)
+	for _, logData := range logs {
+		logsMap[logData.TxHash] = logData


nil check here?

iulianpascalau · 2021-12-23T12:05:53Z

dblookupext/esdtSupply/logsGetter.go

@@ -90,22 +91,3 @@ func (lg *logsGetter) getTxLog(txHash []byte) (data.LogHandler, bool, error) {

 	return logFromDB, true, nil
 }
-
-func mergeLogsSlices(m1, m2 []indexer.LogData) []indexer.LogData {


iulianpascalau · 2021-12-23T12:06:34Z

dblookupext/esdtSupply/logsProcessor.go

@@ -55,6 +54,9 @@ func (lp *logsProcessor) processLogs(blockNonce uint64, logs []indexer.LogData,

 	supplies := make(map[string]*SupplyESDT)
 	for _, logHandler := range logs {
+		if logHandler == nil {


could have been: if logHandler == nil || check.IfNil(logHandler.LogHandler) {

iulianpascalau · 2021-12-23T12:07:04Z

go.sum

-github.com/ElrondNetwork/covalent-indexer-go v1.0.5-0.20211221133537-5f9e7df77444/go.mod h1:dZGbdYuMcIx+E4N/FRT+k+fs0G/+OV4V6bhB58QpUEc=
-github.com/ElrondNetwork/elastic-indexer-go v1.1.29-0.20211221131443-f8a5ee61cac4 h1:59JdRRT4Ydx97Og6iLe79xmwuVBD4P9IVGK9VVl8OFQ=
-github.com/ElrondNetwork/elastic-indexer-go v1.1.29-0.20211221131443-f8a5ee61cac4/go.mod h1:nkcuucWvT9D+iEihxXLXGiK/Cach1tFm3TAj9gFIUcQ=
+github.com/ElrondNetwork/covalent-indexer-go v1.0.5-0.20211222142224-033f6d346e44 h1:jNeBuGfjiqNN+zFOroUiP0D+fIYfhpVk+jSHEBaYkeQ=


proper releases

after everything is tested 👍

iulianpascalau · 2021-12-23T12:11:46Z

process/transactionLog/process.go

@@ -98,19 +97,19 @@ func (tlp *txLogProcessor) GetLogFromCache(txHash []byte) (indexer.LogData, bool

 	txLog, err := tlp.GetLog(txHash)


could have made the logsIndices actually a map of logData and thus, removing the slice indexing on L95

I would leave it as it is as we want to make sure of the order

Irrelevant: the map could have been used just to avoid double reads: one from a map and one from the slice

LucianMincu

System tests passed.

change logs from map to slice

3be222f

bogdan-rosianu self-assigned this Dec 21, 2021

bogdan-rosianu and others added 3 commits December 21, 2021 16:50

referenced new notifier and fix golangci warning

654ebf8

fix after self review

358050e

Merge branch 'development' into change-logs-map-to-slice

48cbed1

bogdan-rosianu marked this pull request as ready for review December 21, 2021 15:36

iulianpascalau self-requested a review December 21, 2021 16:01

ssd04 self-requested a review December 21, 2021 16:07

iulianpascalau requested changes Dec 21, 2021

View reviewed changes

bogdan-rosianu added 4 commits December 22, 2021 13:30

fixes after review

386ec97

Merge remote-tracking branch 'origin/change-logs-map-to-slice' into c…

44ff85e

…hange-logs-map-to-slice

reference new elrond-go-core

c3c03ef

use proper core release

d5067ac

ssd04 reviewed Dec 22, 2021

View reviewed changes

dblookupext/historyRepository.go Outdated Show resolved Hide resolved

ssd04 reviewed Dec 22, 2021

View reviewed changes

bogdan-rosianu added 3 commits December 22, 2021 16:27

small fixes

01503cd

reference new indexer

b34d88f

Merge branch 'development' into change-logs-map-to-slice

67e5c83

# Conflicts: # go.mod # go.sum

ssd04 reviewed Dec 22, 2021

View reviewed changes

ssd04 previously approved these changes Dec 22, 2021

View reviewed changes

Merge branch 'development' into change-logs-map-to-slice

1918ca6

iulianpascalau reviewed Dec 23, 2021

View reviewed changes

LucianMincu and others added 2 commits December 23, 2021 15:27

Merge branch 'development' into change-logs-map-to-slice

7b2060d

fixes after review

b79ebb1

bogdan-rosianu dismissed ssd04’s stale review via b79ebb1 December 23, 2021 14:37

ssd04 previously approved these changes Dec 24, 2021

View reviewed changes

use proper releases

046d0e2

bogdan-rosianu dismissed ssd04’s stale review via 046d0e2 December 24, 2021 08:33

ssd04 approved these changes Dec 24, 2021

View reviewed changes

LucianMincu approved these changes Dec 24, 2021

View reviewed changes

iulianpascalau approved these changes Dec 27, 2021

View reviewed changes

iulianpascalau merged commit e6ef82a into development Dec 27, 2021

iulianpascalau deleted the change-logs-map-to-slice branch December 27, 2021 06:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change logs from map to slice #3655

Change logs from map to slice #3655

bogdan-rosianu commented Dec 21, 2021

codecov bot commented Dec 21, 2021 •

edited

iulianpascalau Dec 21, 2021

iulianpascalau Dec 21, 2021

bogdan-rosianu Dec 22, 2021

ssd04 Dec 22, 2021

bogdan-rosianu Dec 22, 2021

ssd04 Dec 22, 2021

bogdan-rosianu Dec 22, 2021

iulianpascalau Dec 23, 2021

bogdan-rosianu Dec 23, 2021

iulianpascalau Dec 23, 2021

iulianpascalau Dec 23, 2021

bogdan-rosianu Dec 23, 2021

iulianpascalau Dec 23, 2021

bogdan-rosianu Dec 23, 2021

iulianpascalau Dec 23, 2021

bogdan-rosianu Dec 23, 2021

iulianpascalau Dec 23, 2021

LucianMincu left a comment

		@@ -98,19 +97,19 @@ func (tlp *txLogProcessor) GetLogFromCache(txHash []byte) (indexer.LogData, bool

		txLog, err := tlp.GetLog(txHash)

Change logs from map to slice #3655

Change logs from map to slice #3655

Conversation

bogdan-rosianu commented Dec 21, 2021

codecov bot commented Dec 21, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LucianMincu left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 21, 2021 •

edited