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 new PoET config "poet-request-timeout" and use it instead of GracePeriod for timeouts #4996

Closed
wants to merge 6 commits into from

Conversation

fasmat
Copy link
Member

@fasmat fasmat commented Sep 11, 2023

Motivation

The NiPoST builder uses GracePeriod for two different (unrelated) things: one is a duration relative to a PoET round start. A node waits until then before it starts creating and submitting a challenge to PoET (so that it uses the highest possible ATX it sees until then).

The other use is as a request timeout for requests to PoET. For this a new config parameter "poet-request-timeout" is added to the config. This allows to set much shorter timeouts (1 hour on mainnet is much too long).

Changes

  • add new config parameter "poet-request-timeout"
  • the defaults are calculated from the maximum time a request can take based on the linear jitter backoff

Test Plan

existing tests pass

TODO

  • Explain motivation or link existing issue(s)
  • Test changes and document test plan
  • Update documentation as needed
  • Update changelog as needed

@fasmat fasmat self-assigned this Sep 11, 2023
@fasmat fasmat force-pushed the rename-grace-period branch 3 times, most recently from 44eb577 to cdf8cf9 Compare September 11, 2023 17:04
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #4996 (3a46a1d) into develop (6659ac0) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           develop   #4996   +/-   ##
=======================================
  Coverage     77.1%   77.1%           
=======================================
  Files          254     254           
  Lines        30296   30301    +5     
=======================================
+ Hits         23363   23377   +14     
+ Misses        5416    5404   -12     
- Partials      1517    1520    +3     
Files Changed Coverage Δ
activation/activation.go 75.5% <100.0%> (-0.2%) ⬇️
activation/nipost.go 81.5% <100.0%> (+0.5%) ⬆️
config/mainnet.go 95.7% <100.0%> (+<0.1%) ⬆️
config/presets/fastnet.go 100.0% <100.0%> (ø)
config/presets/standalone.go 100.0% <100.0%> (ø)

... and 5 files with indirect coverage changes

@fasmat
Copy link
Member Author

fasmat commented Sep 11, 2023

bors try

bors bot added a commit that referenced this pull request Sep 11, 2023
@fasmat
Copy link
Member Author

fasmat commented Sep 11, 2023

bors try

@bors
Copy link

bors bot commented Sep 11, 2023

try

Already running a review

@bors
Copy link

bors bot commented Sep 11, 2023

try

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.

@fasmat fasmat force-pushed the rename-grace-period branch from 05e5f75 to 4eed88f Compare September 12, 2023 13:46
config/mainnet.go Outdated Show resolved Hide resolved
activation/nipost.go Outdated Show resolved Hide resolved
@fasmat fasmat force-pushed the rename-grace-period branch from e2736c8 to cab09ff Compare September 12, 2023 15:23
@fasmat
Copy link
Member Author

fasmat commented Sep 12, 2023

bors merge

bors bot pushed a commit that referenced this pull request Sep 12, 2023
…ePeriod for timeouts (#4996)

## Motivation
The NiPoST builder uses `GracePeriod` for two different (unrelated) things: one is a duration relative to a PoET round start. A node waits until then before it starts creating and submitting a challenge to PoET (so that it uses the highest possible ATX it sees until then).

The other use is as a request timeout for requests to PoET. For this a new config parameter "poet-request-timeout" is added to the config. This allows to set much shorter timeouts (1 hour on mainnet is much too long).

## Changes
- add new config parameter "poet-request-timeout"
- the defaults are calculated from the maximum time a request can take based on the linear jitter backoff

## Test Plan
existing tests pass

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [x] Update documentation as needed
- [x] Update [changelog](../CHANGELOG.md) as needed
@bors
Copy link

bors bot commented Sep 12, 2023

Build failed:

@fasmat
Copy link
Member Author

fasmat commented Sep 12, 2023

bors merge

bors bot pushed a commit that referenced this pull request Sep 12, 2023
…ePeriod for timeouts (#4996)

## Motivation
The NiPoST builder uses `GracePeriod` for two different (unrelated) things: one is a duration relative to a PoET round start. A node waits until then before it starts creating and submitting a challenge to PoET (so that it uses the highest possible ATX it sees until then).

The other use is as a request timeout for requests to PoET. For this a new config parameter "poet-request-timeout" is added to the config. This allows to set much shorter timeouts (1 hour on mainnet is much too long).

## Changes
- add new config parameter "poet-request-timeout"
- the defaults are calculated from the maximum time a request can take based on the linear jitter backoff

## Test Plan
existing tests pass

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [x] Update documentation as needed
- [x] Update [changelog](../CHANGELOG.md) as needed
@bors
Copy link

bors bot commented Sep 12, 2023

Build failed:

@fasmat
Copy link
Member Author

fasmat commented Sep 12, 2023

bors merge

bors bot pushed a commit that referenced this pull request Sep 12, 2023
…ePeriod for timeouts (#4996)

## Motivation
The NiPoST builder uses `GracePeriod` for two different (unrelated) things: one is a duration relative to a PoET round start. A node waits until then before it starts creating and submitting a challenge to PoET (so that it uses the highest possible ATX it sees until then).

The other use is as a request timeout for requests to PoET. For this a new config parameter "poet-request-timeout" is added to the config. This allows to set much shorter timeouts (1 hour on mainnet is much too long).

## Changes
- add new config parameter "poet-request-timeout"
- the defaults are calculated from the maximum time a request can take based on the linear jitter backoff

## Test Plan
existing tests pass

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [x] Update documentation as needed
- [x] Update [changelog](../CHANGELOG.md) as needed
@bors
Copy link

bors bot commented Sep 12, 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 new PoET config "poet-request-timeout" and use it instead of GracePeriod for timeouts [Merged by Bors] - Add new PoET config "poet-request-timeout" and use it instead of GracePeriod for timeouts Sep 12, 2023
@bors bors bot closed this Sep 12, 2023
@bors bors bot deleted the rename-grace-period branch September 12, 2023 20:29
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.

2 participants