Tweak config params #2206

iulianpascalau · 2020-08-06T11:10:22Z

tweaked config.toml values for a better sync process
removed a bootstorer bug that prevented the node to start correctly from storage if it had incomplete information
added histogram prints in p2p regarding the peers split on shards

Manual testing procedures:

since the new config values accentuates the bootstorer bug, all we need to test is the normal manual testing of this implementation. The problem arrived on random nodes in low value epochs (after a maximum of 4 epochs we had stuck nodes), so testing on the order of tens of epoch should suffice.

- updated MaxNumberOfNodesForStake to the mainnet value in order to better test the new branches

sasurobert · 2020-08-06T11:12:49Z

cmd/node/config/systemSmartContractsConfig.toml

@@ -8,8 +8,8 @@
    NumRoundsWithoutBleed = 100
    MaximumPercentageToBleed = 0.5
    BleedPercentagePerRound = 0.00001
-    MaxNumberOfNodesForStake = 28
-    NodesToSelectInAuction = 28


Moved if back to 28

@LucianMincu needs to test this in "as close as it can get to mainnet" scenario

SebastianMarian

We should test this PR in two different edge cases for which these values have been set to 1. (the most important thing is to keep at least the BatchDelaySeconds = 2). So, the two edge cases are:

First is when all the nodes (from one shard or all shards) are stopped in almost the same time but before they had the chance to put the batch in the storage and the cacher would not be flushed in. Then the nodes could start behind final and if they would have a majority, and in this case they would certainly have, they would propose another block which is already finalized and broadcasted to meta/shards.

The second problem is with MetaBlockStorage used by shards when they finalize all the miniblocks from one metablock. Then, and only then, they will put the meta in the storage. If they are roll back or restart from storage, they will load the previous/last info from storage and based on which meta blocks are found in storage they can correctly set which metablocks and miniblocks are finalized and which are not yet. If this info is not found in storage, but only in batch, and getting stuff from batch is not working (now as far as I remember this should be fixed) or the node is stopped unexpectedly and the cacher is not flushed into the persister the shard node will have a problem to start and to sync, as it will not set correctly the already processed miniblocks from finalized metablocks.

iulianpascalau · 2020-08-06T12:32:36Z

We should test this PR in two different edge cases for which these values have been set to 1. (the most important thing is to keep at least the BatchDelaySeconds = 2). So, the two edge cases are:

First is when all the nodes (from one shard or all shards) are stopped in almost the same time but before they had the chance to put the batch in the storage and the cacher would not be flushed in. Then the nodes could start behind final and if they would have a majority, and in this case they would certainly have, they would propose another block which is already finalized and broadcasted to meta/shards.

The second problem is with MetaBlockStorage used by shards when they finalize all the miniblocks from one metablock. Then, and only then, they will put the meta in the storage. If they are roll back or restart from storage, they will load the previous/last info from storage and based on which meta blocks are found in storage they can correctly set which metablocks and miniblocks are finalized and which are not yet. If this info is not found in storage, but only in batch, and getting stuff from batch is not working (now as far as I remember this should be fixed) or the node is stopped unexpectedly and the cacher is not flushed into the persister the shard node will have a problem to start and to sync, as it will not set correctly the already processed miniblocks from finalized metablocks.

Totally agree. The BatchDelaySeconds will be kept to 2 seconds for above mentioned reasons.

…strap from storage

…k/elrond-go into tweak-config-params

…n headers

sasurobert · 2020-08-13T10:34:23Z

process/block/baseProcess.go

@@ -794,6 +794,15 @@ func (bp *baseProcessor) prepareDataForBootStorer(args bootStorerDataArgs) {
 		EpochStartTriggerConfigKey: args.epochStartTriggerConfigKey,
 	}

+	log.Debug("baseProcessor.prepareDataForBootStorer",


this is called in every commit block - is it not too much data for log.Debug ?

yeah, was used in debugging

sasurobert · 2020-08-13T10:34:34Z

process/block/baseProcess.go

+	for _, h := range hdrs {
+		str += bp.displayBoostrapHeaderInfo(h)
+	}
+


delete empty line

removed function -> no longer necessary

sasurobert · 2020-08-13T10:34:55Z

process/sync/storageBootstrap/baseStorageBootstrapper.go

+	for _, h := range hdrs {
+		str += st.displayBoostrapHeaderInfo(h)
+	}
+


delete empty line

removed function -> no longer necessary

LucianMincu

System tests passed.

- tweaked config.toml values for a better sync process

7d79952

- updated MaxNumberOfNodesForStake to the mainnet value in order to better test the new branches

iulianpascalau requested a review from SebastianMarian August 6, 2020 11:10

sasurobert requested changes Aug 6, 2020

View reviewed changes

AdoAdoAdo previously approved these changes Aug 6, 2020

View reviewed changes

sasurobert previously approved these changes Aug 6, 2020

View reviewed changes

iulianpascalau self-assigned this Aug 6, 2020

SebastianMarian previously approved these changes Aug 6, 2020

View reviewed changes

iulianpascalau and others added 4 commits August 6, 2020 17:18

Merge branch 'development' into tweak-config-params

50dc3b1

Merge branch 'development' into tweak-config-params

6b13d00

Merge branch 'development' into tweak-config-params

d7bf962

- added bootstorer prints

2acc6de

iulianpascalau dismissed stale reviews from SebastianMarian, sasurobert, and AdoAdoAdo via 2acc6de August 10, 2020 13:10

iulianpascalau and others added 6 commits August 11, 2020 11:47

- added p2p prints

d078b5a

Merge branch 'development' into tweak-config-params

8c27af2

- added protection to load all involved cross headers when doing boot…

1c89269

…strap from storage

Merge branch 'tweak-config-params' of https://github.com/ElrondNetwor…

a738f9b

…k/elrond-go into tweak-config-params

- fixed integration test

d7c666d

- removed an unwanted protection against the cross notarized metachai…

b93aaa0

…n headers

iulianpascalau mentioned this pull request Aug 12, 2020

[BUG] v1.1.0 validator node lost sync then got stuck in fork detector loop #2210

Closed

LucianMincu added 2 commits August 13, 2020 10:46

Merge branch 'development' into tweak-config-params

9906df9

Merge branch 'development' into tweak-config-params

8e12d1e

sasurobert requested changes Aug 13, 2020

View reviewed changes

AdoAdoAdo previously approved these changes Aug 13, 2020

View reviewed changes

- removed the bootstorer print when committing a new block

8440061

iulianpascalau dismissed AdoAdoAdo’s stale review via 8440061 August 13, 2020 11:01

AdoAdoAdo approved these changes Aug 13, 2020

View reviewed changes

sasurobert approved these changes Aug 13, 2020

View reviewed changes

SebastianMarian approved these changes Aug 13, 2020

View reviewed changes

LucianMincu approved these changes Aug 13, 2020

View reviewed changes

LucianMincu merged commit d331516 into development Aug 13, 2020

LucianMincu deleted the tweak-config-params branch August 13, 2020 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak config params #2206

Tweak config params #2206

iulianpascalau commented Aug 6, 2020 •

edited

sasurobert Aug 6, 2020

iulianpascalau Aug 6, 2020

SebastianMarian left a comment

iulianpascalau commented Aug 6, 2020

sasurobert Aug 13, 2020

iulianpascalau Aug 13, 2020

iulianpascalau Aug 13, 2020

sasurobert Aug 13, 2020

iulianpascalau Aug 13, 2020

sasurobert Aug 13, 2020

iulianpascalau Aug 13, 2020

LucianMincu left a comment

Tweak config params #2206

Tweak config params #2206

Conversation

iulianpascalau commented Aug 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SebastianMarian left a comment

Choose a reason for hiding this comment

iulianpascalau commented Aug 6, 2020

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

iulianpascalau commented Aug 6, 2020 •

edited