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

[Merged by Bors] - Add jitter to spread out requests to get poet proof and submit challenge #4871

Closed
wants to merge 4 commits into from

Conversation

poszu
Copy link
Contributor

@poszu poszu commented Aug 18, 2023

Motivation

Closes #4860

Changes

Add small positive jitter when:

  • waiting for a poet round to end (to get proof),
  • waiting to build a nipost challenge (to spread out challenge registrations).
    Jitter is only added if the round end is in the future to avoid unnecessary wait.

The range of jitter duration is calculated as a percentage of a cycle gap duration so it also works in unit tests and system tests.

Jitter before getting the proof

The min and max % (0.02% and 0.04%) were chosen so that on mainnet:

  • minimum jitter is roughly equivalent to the time it takes to generate a proof on bare metal poets (it happens after the round ends): 8.64s. This helps avoid situations when many nodes got a 404 response.
  • maximum jitter is 2xmin - roughly 17s

Jitter before submitting challenges

on mainnet:

  • min = 0s
  • max = 36s (1% of grace period)

Test Plan

Added unit tests checking calculating wait and jitter times.

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #4871 (a4d7bb3) into develop (bfa734e) will decrease coverage by 0.3%.
Report is 12 commits behind head on develop.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           develop   #4871     +/-   ##
=========================================
- Coverage     76.9%   76.7%   -0.3%     
=========================================
  Files          261     261             
  Lines        29807   30241    +434     
=========================================
+ Hits         22951   23222    +271     
- Misses        5392    5519    +127     
- Partials      1464    1500     +36     
Files Changed Coverage Δ
activation/activation.go 76.0% <100.0%> (+0.1%) ⬆️
activation/nipost.go 84.3% <100.0%> (+0.1%) ⬆️

... and 36 files with indirect coverage changes

activation/nipost.go Outdated Show resolved Hide resolved
activation/nipost.go Outdated Show resolved Hide resolved
activation/nipost.go Outdated Show resolved Hide resolved
activation/nipost.go Outdated Show resolved Hide resolved
// We add a jitter to avoid all nodes querying for the proof at the same time.
func calcGetProofWaitTime(tillRoundEnd, cycleGap time.Duration) (waitTime time.Duration) {
minWaitTime := time.Duration(float64(cycleGap) * MinPoetGetProofJitter / 100.0)
maxWaitTime := time.Duration(float64(cycleGap) * MaxPoetGetProofJitter / 100.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

so thats 0.04% and not 4%? i got confused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's 0.02% and 0.04%. I can change it if it's confusing.

Jitter is within [0; 1% of grace period].
@poszu poszu changed the title Add jitter to spread out requests to get poet proof Add jitter to spread out requests to get poet proof and submit challenge Aug 21, 2023
@poszu poszu requested review from fasmat and dshulyak August 21, 2023 12:23
@poszu
Copy link
Contributor Author

poszu commented Aug 21, 2023

bors merge

@poszu
Copy link
Contributor Author

poszu commented Aug 21, 2023

bors cancel

@bors
Copy link

bors bot commented Aug 21, 2023

Canceled.

@poszu
Copy link
Contributor Author

poszu commented Aug 21, 2023

bors merge

bors bot pushed a commit that referenced this pull request Aug 21, 2023
…nge (#4871)

## Motivation
Closes #4860 

## Changes
Add small positive jitter when:
- waiting for a poet round to end (to get proof),
- waiting to build a nipost challenge (to spread out challenge registrations).
Jitter is only added if the round end is in the future to avoid unnecessary wait.

The range of jitter duration is calculated as a percentage of a cycle gap duration so it also works in unit tests and system tests. 

### Jitter before getting the proof
The min and max % (0.02% and 0.04%) were chosen so that on mainnet:
- minimum jitter is roughly equivalent to the time it takes to generate a proof on bare metal poets (it happens after the round ends): 8.64s. This helps avoid situations when many nodes got a 404 response.
- maximum jitter is 2xmin - roughly 17s

### Jitter before submitting challenges
on mainnet:
- min = 0s
- max = 36s (1% of grace period)

## Test Plan
Added unit tests checking calculating wait and jitter times.
@bors
Copy link

bors bot commented Aug 21, 2023

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Add jitter to spread out requests to get poet proof and submit challenge [Merged by Bors] - Add jitter to spread out requests to get poet proof and submit challenge Aug 21, 2023
@bors bors bot closed this Aug 21, 2023
@bors bors bot deleted the 4860-jitterize-requests-to-poet branch August 21, 2023 15:39
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.

Add small jitter to requests to poet
3 participants