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

sharding: balance shards waiting lists with shuffled out nodes #2950

Merged
merged 10 commits into from
Apr 12, 2021

Conversation

AdoAdoAdo
Copy link
Contributor

with activation of capped shuffling of nodes, the shards waiting lists may become unbalanced
this may cause waiting times to differ between shards waiting lists
this PR will try at the end of each epoch to first equalize the waiting lists.

@AdoAdoAdo AdoAdoAdo marked this pull request as draft March 31, 2021 16:08
@codecov
Copy link

codecov bot commented Mar 31, 2021

Codecov Report

Merging #2950 (f04b74f) into master (82c7a3f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head f04b74f differs from pull request most recent head b60f782. Consider uploading reports for the commit b60f782 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2950      +/-   ##
==========================================
+ Coverage   75.01%   75.02%   +0.01%     
==========================================
  Files         620      620              
  Lines       59062    59086      +24     
==========================================
+ Hits        44305    44329      +24     
  Misses      10803    10803              
  Partials     3954     3954              
Impacted Files Coverage Δ
sharding/hashValidatorShuffler.go 91.63% <100.00%> (+0.69%) ⬆️
sharding/validatorDistributor.go 100.00% <100.00%> (ø)

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 82c7a3f...b60f782. Read the comment docs.

@AdoAdoAdo AdoAdoAdo self-assigned this Apr 1, 2021
@AdoAdoAdo AdoAdoAdo marked this pull request as ready for review April 1, 2021 09:36
@sasurobert sasurobert self-requested a review April 2, 2021 06:35
err = distributeValidators(newWaiting, arg.newNodes, arg.randomness)

err = distributeValidators(newWaiting, arg.newNodes, arg.randomness, false)

Copy link
Contributor

Choose a reason for hiding this comment

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

delete empty line

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

@iulianpascalau iulianpascalau self-requested a review April 2, 2021 16:24
@@ -657,6 +706,9 @@ func (rhs *randHashShuffler) UpdateShufflerConfig(epoch uint32) {
"epochEnable", rhs.activeNodesConfig.EpochEnable,
"maxNodesToShufflePerShard", rhs.activeNodesConfig.NodesToShufflePerShard,
)

rhs.flagBalanceWaitingLists.Toggle(epoch >= rhs.balanceWaitingListsEnableEpoch)
log.Trace("balanced waiting lists", "enabled", rhs.flagBalanceWaitingLists.IsSet())
Copy link
Contributor

Choose a reason for hiding this comment

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

log.Debug as to align with the rest of the project.

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
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

System test passed

@iulianpascalau iulianpascalau merged commit 3d326fa into master Apr 12, 2021
@iulianpascalau iulianpascalau deleted the balance-shards-waiting-lists branch April 12, 2021 08:52
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