-
Notifications
You must be signed in to change notification settings - Fork 215
EN-13432: Nodes shuffler refactoring #4799
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
Merged
bogdan-rosianu
merged 4 commits into
feat/consensus-size-changes
from
EN-13432-shuffler-refactoring
Dec 21, 2022
Merged
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
156b5ae
EN-13432: shuffler refactoring
bogdan-rosianu 3bb1758
Merge branch 'feat/consensus-size-changes' into EN-13432-shuffler-ref…
bogdan-rosianu 1fad090
EN-13432: fixes after review:
bogdan-rosianu 7efe443
EN-13432: stub and mock cleanup
bogdan-rosianu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,13 +18,10 @@ var _ NodesShuffler = (*randHashShuffler)(nil) | |
|
|
||
| // NodesShufflerArgs defines the arguments required to create a nodes shuffler | ||
| type NodesShufflerArgs struct { | ||
| NodesShard uint32 | ||
| NodesMeta uint32 | ||
| Hysteresis float32 | ||
| Adaptivity bool | ||
| ShuffleBetweenShards bool | ||
| MaxNodesEnableConfig []config.MaxNodesChangeConfig | ||
| EnableEpochsHandler common.EnableEpochsHandler | ||
| ShuffleBetweenShards bool | ||
| MaxNodesEnableConfig []config.MaxNodesChangeConfig | ||
| EnableEpochsHandler common.EnableEpochsHandler | ||
| ChainParametersHandler ChainParametersHandler | ||
| } | ||
|
|
||
| type shuffleNodesArg struct { | ||
|
|
@@ -49,11 +46,9 @@ type randHashShuffler struct { | |
| // when reinitialization of node in new shard is implemented | ||
| shuffleBetweenShards bool | ||
|
|
||
| adaptivity bool | ||
| nodesShard uint32 | ||
| nodesMeta uint32 | ||
| shardHysteresis uint32 | ||
| metaHysteresis uint32 | ||
| currentChainParameters config.ChainParametersByEpochConfig | ||
| //shardHysteresis uint32 | ||
|
||
| //metaHysteresis uint32 | ||
| activeNodesConfig config.MaxNodesChangeConfig | ||
| availableNodesConfigs []config.MaxNodesChangeConfig | ||
| mutShufflerParams sync.RWMutex | ||
|
|
@@ -72,7 +67,9 @@ func NewHashValidatorsShuffler(args *NodesShufflerArgs) (*randHashShuffler, erro | |
| if check.IfNil(args.EnableEpochsHandler) { | ||
| return nil, ErrNilEnableEpochsHandler | ||
| } | ||
|
|
||
| if check.IfNil(args.ChainParametersHandler) { | ||
| return nil, ErrNilChainParametersHandler | ||
| } | ||
| var configs []config.MaxNodesChangeConfig | ||
|
|
||
| log.Debug("hashValidatorShuffler: enable epoch for max nodes change", "epoch", args.MaxNodesEnableConfig) | ||
|
|
@@ -81,15 +78,15 @@ func NewHashValidatorsShuffler(args *NodesShufflerArgs) (*randHashShuffler, erro | |
| copy(configs, args.MaxNodesEnableConfig) | ||
| } | ||
|
|
||
| currentChainParameters := args.ChainParametersHandler.CurrentChainParameters() | ||
| log.Debug("Shuffler created", "shuffleBetweenShards", args.ShuffleBetweenShards) | ||
| rxs := &randHashShuffler{ | ||
| shuffleBetweenShards: args.ShuffleBetweenShards, | ||
| availableNodesConfigs: configs, | ||
| enableEpochsHandler: args.EnableEpochsHandler, | ||
| currentChainParameters: currentChainParameters, | ||
| shuffleBetweenShards: args.ShuffleBetweenShards, | ||
| availableNodesConfigs: configs, | ||
| enableEpochsHandler: args.EnableEpochsHandler, | ||
| } | ||
|
|
||
| rxs.UpdateParams(args.NodesShard, args.NodesMeta, args.Hysteresis, args.Adaptivity) | ||
|
|
||
| if rxs.shuffleBetweenShards { | ||
| rxs.validatorDistributor = &CrossShardValidatorDistributor{} | ||
| } else { | ||
|
|
@@ -101,27 +98,6 @@ func NewHashValidatorsShuffler(args *NodesShufflerArgs) (*randHashShuffler, erro | |
| return rxs, nil | ||
| } | ||
|
|
||
| // UpdateParams updates the shuffler parameters | ||
| // Should be called when new params are agreed through governance | ||
| func (rhs *randHashShuffler) UpdateParams( | ||
| nodesShard uint32, | ||
| nodesMeta uint32, | ||
| hysteresis float32, | ||
| adaptivity bool, | ||
| ) { | ||
| // TODO: are there constraints we want to enforce? e.g min/max hysteresis | ||
| shardHysteresis := uint32(float32(nodesShard) * hysteresis) | ||
| metaHysteresis := uint32(float32(nodesMeta) * hysteresis) | ||
|
|
||
| rhs.mutShufflerParams.Lock() | ||
| rhs.shardHysteresis = shardHysteresis | ||
| rhs.metaHysteresis = metaHysteresis | ||
| rhs.nodesShard = nodesShard | ||
| rhs.nodesMeta = nodesMeta | ||
| rhs.adaptivity = adaptivity | ||
| rhs.mutShufflerParams.Unlock() | ||
| } | ||
|
|
||
| // UpdateNodeLists shuffles the nodes and returns the lists with the new nodes configuration | ||
| // The function needs to ensure that: | ||
| // 1. Old eligible nodes list will have up to shuffleOutThreshold percent nodes shuffled out from each shard | ||
|
|
@@ -143,7 +119,7 @@ func (rhs *randHashShuffler) UpdateNodeLists(args ArgsUpdateNodes) (*ResUpdateNo | |
| eligibleAfterReshard := copyValidatorMap(args.Eligible) | ||
| waitingAfterReshard := copyValidatorMap(args.Waiting) | ||
|
|
||
| args.AdditionalLeaving = removeDupplicates(args.UnStakeLeaving, args.AdditionalLeaving) | ||
| args.AdditionalLeaving = removeDuplicates(args.UnStakeLeaving, args.AdditionalLeaving) | ||
| totalLeavingNum := len(args.AdditionalLeaving) + len(args.UnStakeLeaving) | ||
|
|
||
| newNbShards := rhs.computeNewShards( | ||
|
|
@@ -155,10 +131,10 @@ func (rhs *randHashShuffler) UpdateNodeLists(args ArgsUpdateNodes) (*ResUpdateNo | |
| ) | ||
|
|
||
| rhs.mutShufflerParams.RLock() | ||
| canSplit := rhs.adaptivity && newNbShards > args.NbShards | ||
| canMerge := rhs.adaptivity && newNbShards < args.NbShards | ||
| nodesPerShard := rhs.nodesShard | ||
| nodesMeta := rhs.nodesMeta | ||
| canSplit := rhs.currentChainParameters.Adaptivity && newNbShards > args.NbShards | ||
| canMerge := rhs.currentChainParameters.Adaptivity && newNbShards < args.NbShards | ||
| nodesPerShard := rhs.currentChainParameters.ShardMinNumNodes | ||
| nodesMeta := rhs.currentChainParameters.MetachainMinNumNodes | ||
| rhs.mutShufflerParams.RUnlock() | ||
|
|
||
| if canSplit { | ||
|
|
@@ -185,7 +161,7 @@ func (rhs *randHashShuffler) UpdateNodeLists(args ArgsUpdateNodes) (*ResUpdateNo | |
| }) | ||
| } | ||
|
|
||
| func removeDupplicates(unstake []Validator, additionalLeaving []Validator) []Validator { | ||
| func removeDuplicates(unstake []Validator, additionalLeaving []Validator) []Validator { | ||
| additionalCopy := make([]Validator, 0, len(additionalLeaving)) | ||
| additionalCopy = append(additionalCopy, additionalLeaving...) | ||
|
|
||
|
|
@@ -452,10 +428,10 @@ func (rhs *randHashShuffler) computeNewShards( | |
| nodesNewEpoch := uint32(nbEligible + nbWaiting + numNewNodes - numLeavingNodes) | ||
|
|
||
| rhs.mutShufflerParams.RLock() | ||
| maxNodesMeta := rhs.nodesMeta + rhs.metaHysteresis | ||
| maxNodesShard := rhs.nodesShard + rhs.shardHysteresis | ||
| maxNodesMeta := rhs.currentChainParameters.MetachainMinNumNodes + rhs.metaHysteresis() | ||
| maxNodesShard := rhs.currentChainParameters.ShardMinNumNodes + rhs.shardHysteresis() | ||
| nodesForSplit := (nbShards+1)*maxNodesShard + maxNodesMeta | ||
| nodesForMerge := nbShards*rhs.nodesShard + rhs.nodesMeta | ||
| nodesForMerge := nbShards*rhs.currentChainParameters.ShardMinNumNodes + rhs.currentChainParameters.MetachainMinNumNodes | ||
| rhs.mutShufflerParams.RUnlock() | ||
|
|
||
| nbShardsNew := nbShards | ||
|
|
@@ -473,6 +449,14 @@ func (rhs *randHashShuffler) computeNewShards( | |
| return nbShardsNew | ||
| } | ||
|
|
||
| func (rhs *randHashShuffler) metaHysteresis() uint32 { | ||
| return uint32(rhs.currentChainParameters.Hysteresis * float32(rhs.currentChainParameters.MetachainMinNumNodes)) | ||
| } | ||
|
|
||
| func (rhs *randHashShuffler) shardHysteresis() uint32 { | ||
| return uint32(rhs.currentChainParameters.Hysteresis * float32(rhs.currentChainParameters.ShardMinNumNodes)) | ||
| } | ||
|
|
||
| // shuffleOutNodes shuffles the list of eligible validators in each shard and returns the map of shuffled out | ||
| // validators | ||
| func shuffleOutNodes( | ||
|
|
@@ -761,7 +745,7 @@ func sortKeys(nodes map[uint32][]Validator) []uint32 { | |
| func (rhs *randHashShuffler) UpdateShufflerConfig(epoch uint32) { | ||
| rhs.mutShufflerParams.Lock() | ||
| defer rhs.mutShufflerParams.Unlock() | ||
| rhs.activeNodesConfig.NodesToShufflePerShard = rhs.nodesShard | ||
| rhs.activeNodesConfig.NodesToShufflePerShard = rhs.currentChainParameters.ShardMinNumNodes | ||
| for _, maxNodesConfig := range rhs.availableNodesConfigs { | ||
| if epoch >= maxNodesConfig.EpochEnable { | ||
| rhs.activeNodesConfig = maxNodesConfig | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
check also if the notify order for chain parameters handler.
it needs to at least be before nodes coordinator.
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.
refactored - so that it uses the epoch notifier with ordering