Exchanges integration storers #2244

andreibancioiu · 2020-08-26T22:37:30Z

After restarting a node, GetFromEpoch could not access persisters older than NumActivePersisters.

Fix: when CleanOldEpochsData is unset, then open all persisters (and close old ones, as before)
Fix: call setIsClosed() when closing the persisters

In the PR 2226, I've mistakenly disabled epoch by tx hash indexing (and thus get tx by hash API wouldn't work for older epochs). Re-made now the link to epoch, by making MiniblockHashByTxHashStorage Static. Put transactions is fast (1 put per transaction), get 1 transaction is slower (3 reads).
Index genesis block (for hyperblock nonce 1).
Fixed path for EpochByHashStorage. FilePath = "EpochByHash", it was empty due to PR 2226.
Fixed edge-case for saving hyperblock coordinates of cross-epoch (committed in 42, notarized in 43) transactions. Using putInEpoch primitive.
Rename "fullHistory" to "db lookup extensions".

How to test

1 and 2 - wait a few epochs, stop some observers, start them again, query older transactions and older (hyper)blocks.

3 - Query hyperblock 1 (by nonce, by hash). Query shard blocks 0 (by nonce, by hash). Compare with Explorer.

4 - Assert that DB folders structure in Static is nice.

5 - A bit harder to test manually - we have unit tests though.

6 - Inspect config.toml and node/db's structure.

Note that, for tests, I've set:

RoundsPerEpoch = 20
FullHistory.Enabled = true

cmd/node/config/config.toml

storage/pruning/pruningStorer.go

iulianpascalau · 2020-08-27T11:01:24Z

storage/pruning/pruningStorer.go

@@ -340,6 +348,7 @@ func (ps *PruningStorer) GetFromEpoch(key []byte, epoch uint32) ([]byte, error)
 	}

 	persister, err := ps.persisterFactory.Create(pd.path)
+	// Question for review: save reference on persister data?


IMHO, I do not think we will need the old closed reference.

iulianpascalau · 2020-08-27T11:01:30Z

storage/pruning/pruningStorer.go

@@ -310,6 +316,8 @@ func (ps *PruningStorer) Close() error {
 			log.Error("cannot close persister", err)
 			closedSuccessfully = false
 		}
+		// Question for review - all good here?
+		persister.setIsClosed(true)


iulianpascalau · 2020-08-27T11:01:37Z

storage/pruning/pruningStorer.go

@@ -350,6 +359,8 @@ func (ps *PruningStorer) GetFromEpoch(key []byte, epoch uint32) ([]byte, error)
 		if err != nil {
 			log.Debug("persister.Close()", "error", err.Error())
 		}
+		// Question for review: all good here?
+		pd.setIsClosed(true)


iulianpascalau · 2020-08-27T11:01:44Z

storage/pruning/pruningStorer.go

@@ -400,6 +412,8 @@ func (ps *PruningStorer) GetBulkFromEpoch(keys [][]byte, epoch uint32) (map[stri
 			if err != nil {
 				log.Debug("persister.Close()", "error", err.Error())
 			}
+			// Question for review - all good here?
+			pd.setIsClosed(true)


iulianpascalau · 2020-08-27T11:01:54Z

storage/pruning/pruningStorer.go

@@ -517,6 +532,8 @@ func (ps *PruningStorer) HasInEpoch(key []byte, epoch uint32) error {
 			if err != nil {
 				log.Debug("persister.Close()", "error", err.Error())
 			}
+			// Question for review - all good here?
+			pd.setIsClosed(true)


iulianpascalau · 2020-08-27T11:02:15Z

storage/pruning/pruningStorer.go

+func createShallowPersisterDataForEpoch(args *StorerArgs, epoch uint32, shardIdStr string) *persisterData {
+	filePath := createPersisterPathForEpoch(args, epoch, shardIdStr)
+
+	p := &persisterData{


return directly?

bogdan-rosianu · 2020-08-27T14:33:09Z

storage/pruning/pruningStorer.go

@@ -245,11 +245,16 @@ func (ps *PruningStorer) Put(key, data []byte) error {
 			log.Debug("active persister not found",
 				"epoch", ps.epochForPutOperation,
 				"used", persisterToUse.epoch)
+
+			// Question for review: isn't this a critical error?


I think it is critical because otherwise you'd save data in a persister which is not for the desired epoch

If we return error here, the node won't advance: this will keep the DB as consistent as possible. If we not, the node will advance, but the data might not be stored inside

Understood. Left as it were - to prevent breaking something unforeseen. However, let me know if I should change instead.

this case didn't happen because the epochForPutOperation is set on CommitBlock so there are very small chances that the persister for the specified epoch doesn't exist. can remain like this for now

…ookupext Rename "fullHistory" to "db lookup extensions"

core/dblookupext/historyRepository.go

node/nodeTransactions_test.go

storage/factory/pruningStorerFactory.go

storage/pruning/pruningStorer.go

iulianpascalau · 2020-08-27T16:03:47Z

storage/pruning/pruningStorer.go

@@ -245,11 +245,16 @@ func (ps *PruningStorer) Put(key, data []byte) error {
 			log.Debug("active persister not found",
 				"epoch", ps.epochForPutOperation,
 				"used", persisterToUse.epoch)
+
+			// Question for review: isn't this a critical error?


If we return error here, the node won't advance: this will keep the DB as consistent as possible. If we not, the node will advance, but the data might not be stored inside

storage/pruning/pruningStorer.go

…ration-storers

…ork/elrond-go into exchanges-integration-storers

storage/pruning/pruningStorer.go

cmd/node/config/config.toml

miiu96 · 2020-08-28T08:34:29Z

cmd/node/factory/structs.go

+			return err
+		}
+
+		log.Info("indexGenesisBlocks(): historyRepo.RecordBlock", "shard", shard, "hash", genesisBlockHash)


is ok this log.Info ? maybe debug ?

Indeed, I wanted it to be more obvious (since it was missing before).

LucianMincu

System tests passed.

andreibancioiu added 9 commits August 26, 2020 19:31

Sketch fix (open and close all persisters).

c8e062b

Config for tests.

8150e42

Add extra logs.

380113a

Bugfix setIsClosed().

29c4899

Bugfix: save epoch by txhash as well (as Mihai originally did)

727a54d

Merge branch 'development' into exchanges-integration-storers

563fd70

Fix tests.

7d6a2fe

Bugfix: get miniblock metadata from epoch storer

cd79ee3

Sketch fix for genesis block indexing (for hyperblock 1).

e470945

andreibancioiu requested review from AdoAdoAdo, iulianpascalau and miiu96 August 26, 2020 22:37

andreibancioiu self-assigned this Aug 26, 2020

Fix after self-review.

dad12f4

andreibancioiu marked this pull request as ready for review August 26, 2020 22:55

Bugfix filepath for EpochByHash storer.

97a4be8

miiu96 previously approved these changes Aug 27, 2020

View reviewed changes

Write many transactions faster, read 1 tx slower

426d78c

andreibancioiu dismissed miiu96’s stale review via 426d78c August 27, 2020 08:39

Fix after self review

dd5e25d

andreibancioiu requested a review from miiu96 August 27, 2020 08:49

AdoAdoAdo approved these changes Aug 27, 2020

View reviewed changes

iulianpascalau reviewed Aug 27, 2020

View reviewed changes

cmd/node/config/config.toml Outdated Show resolved Hide resolved

cmd/node/config/config.toml Outdated Show resolved Hide resolved

storage/pruning/pruningStorer.go Outdated Show resolved Hide resolved

AdoAdoAdo requested changes Aug 27, 2020

View reviewed changes

storage/pruning/pruningStorer.go Show resolved Hide resolved

andreibancioiu added 3 commits August 27, 2020 13:00

Fix name of storer.

9d57b83

Fix after review (part 1).

6a0cf91

Fix after review.

2c49f51

iulianpascalau reviewed Aug 27, 2020

View reviewed changes

Undo resetting "oldestEpochKeep"

b67ef8c

andreibancioiu requested a review from bogdan-rosianu August 27, 2020 12:55

bogdan-rosianu reviewed Aug 27, 2020

View reviewed changes

bogdan-rosianu previously approved these changes Aug 27, 2020

View reviewed changes

Pass flag to pruning storer.

1554c7d

andreibancioiu dismissed bogdan-rosianu’s stale review via 1554c7d August 27, 2020 14:56

andreibancioiu and others added 2 commits August 27, 2020 18:31

Rename "fullHistory" to "dblookupext".

7f02171

Merge pull request #2251 from ElrondNetwork/exchanges-integration-dbl…

de7f337

…ookupext Rename "fullHistory" to "db lookup extensions"

andreibancioiu requested a review from bogdan-rosianu August 27, 2020 15:42

iulianpascalau reviewed Aug 27, 2020

View reviewed changes

andreibancioiu added 3 commits August 27, 2020 19:18

Fix after review.

1cba572

Merge branch 'exchanges-integration-dblookupext' into exchanges-integ…

5c90b2b

…ration-storers

Merge branch 'exchanges-integration-storers' of github.com:ElrondNetw…

7a8ed7d

…ork/elrond-go into exchanges-integration-storers

andreibancioiu requested a review from iulianpascalau August 27, 2020 16:23

andreibancioiu added 2 commits August 27, 2020 20:57

Merge branch 'development' into exchanges-integration-storers

244c325

100 rounds per epoch (for tests).

7fcf4b7

iulianpascalau previously approved these changes Aug 27, 2020

View reviewed changes

AdoAdoAdo requested changes Aug 27, 2020

View reviewed changes

storage/pruning/pruningStorer.go Outdated Show resolved Hide resolved

cmd/node/config/config.toml Outdated Show resolved Hide resolved

andreibancioiu and others added 2 commits August 27, 2020 23:03

Merge branch 'development' into exchanges-integration-storers

b3e5ddb

Undo config.toml.

43e0075

andreibancioiu dismissed iulianpascalau’s stale review via 43e0075 August 27, 2020 20:04

andreibancioiu requested review from AdoAdoAdo and iulianpascalau August 27, 2020 20:04

bogdan-rosianu approved these changes Aug 27, 2020

View reviewed changes

AdoAdoAdo approved these changes Aug 28, 2020

View reviewed changes

miiu96 reviewed Aug 28, 2020

View reviewed changes

miiu96 approved these changes Aug 28, 2020

View reviewed changes

miiu96 and others added 2 commits August 28, 2020 11:44

Merge branch 'development' into exchanges-integration-storers

95207ba

Merge branch 'development' into exchanges-integration-storers

2143225

LucianMincu approved these changes Aug 28, 2020

View reviewed changes

LucianMincu merged commit 252085f into development Aug 28, 2020

LucianMincu deleted the exchanges-integration-storers branch August 28, 2020 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exchanges integration storers #2244

Exchanges integration storers #2244

andreibancioiu commented Aug 26, 2020 •

edited

iulianpascalau Aug 27, 2020

iulianpascalau Aug 27, 2020

iulianpascalau Aug 27, 2020

iulianpascalau Aug 27, 2020

iulianpascalau Aug 27, 2020

iulianpascalau Aug 27, 2020

andreibancioiu Aug 27, 2020

bogdan-rosianu Aug 27, 2020

iulianpascalau Aug 27, 2020

andreibancioiu Aug 27, 2020

bogdan-rosianu Aug 27, 2020

iulianpascalau Aug 27, 2020

miiu96 Aug 28, 2020

andreibancioiu Aug 28, 2020

LucianMincu left a comment

Exchanges integration storers #2244

Exchanges integration storers #2244

Conversation

andreibancioiu commented Aug 26, 2020 • edited

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

andreibancioiu commented Aug 26, 2020 •

edited