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

Remove cpu-effort-percent and replace with new option #1784

Closed
3 of 5 tasks
eosusa opened this issue Oct 17, 2023 · 2 comments · Fixed by #1800 or #1805
Closed
3 of 5 tasks

Remove cpu-effort-percent and replace with new option #1784

eosusa opened this issue Oct 17, 2023 · 2 comments · Fixed by #1800 or #1805
Assignees
Labels
👍 lgtm OCI Work exclusive to OCI team

Comments

@eosusa
Copy link

eosusa commented Oct 17, 2023

Brian's Edits

Because the behavior of cpu-effort-percent has changed significantly, we should create a new option instead.

Suggestion noted on changing the default from 90% to 95%, will consider for a future release as we continue to receive feedback from production.

Decisions:

  • Should we DEPRECATE cpu-effort-percent? No as this is a major release and the impact is limited to failure to start.
  • What should we call the new option? produce-block-offset-ms
  • What should the default value be? 450 ms

Remaining Tasks

Original Report

@eosusa's original title: "Rename cpu-effort-percentage to round-cpu-effort-percentage and default to 95%"

With the 5.0 RC2 release, the cpu-effort-percentage parameter functionality was dramatically changed from not controlling each individual block's cpu effort during signing, but now controls the ENTIRE round's cpu effort for ALL blocks cumulatively for the round.

While this is a great new feature to add, having it reuse the original parameter from previous versions that functioned differently I believe is a mistake and could easily clear up some confusion by simply renaming the new parameter vs.s reusing the original.

Also, on most public networks we operate on, the default of 90% round-cpu-effort-percentage is probably a bit too low as it kicks out the last block before it's scheduled time, which leads to some confusion because blocks are arriving early (and most networks don't have to send last block that aggressively). We've found most networks tend to function properly with the last block coming out about 300ms early, which would be roughly 95% round-cpu-effort-percentage I believe (if I understand it correctly, that is :))

@enf-ci-bot enf-ci-bot moved this to Todo in Team Backlog Oct 17, 2023
@bhazzard bhazzard changed the title Rename cpu-effort-percentage to round-cpu-effort-percentage and default to 95% Deprecate cpu-effort-percentage and replace with new option Oct 17, 2023
@bhazzard bhazzard removed the triage label Oct 17, 2023
@bhazzard bhazzard added this to the Leap v5.0.0-stable milestone Oct 17, 2023
@heifner heifner changed the title Deprecate cpu-effort-percentage and replace with new option Deprecate cpu-effort-percent and replace with new option Oct 17, 2023
@bhazzard bhazzard changed the title Deprecate cpu-effort-percent and replace with new option Remove cpu-effort-percent and replace with new option Oct 17, 2023
@heifner heifner self-assigned this Oct 17, 2023
@heifner heifner moved this from Todo to In Progress in Team Backlog Oct 17, 2023
@heifner heifner added the OCI Work exclusive to OCI team label Oct 17, 2023
@RalfWeinand
Copy link

I suggest to change the new option from a percentage to an offset in ms for better fine-tuning. The name could be something like “produce-block-offset-ms”. [A general suggestion for new parameter names is starting with the name of the plugin that it relates for easier grouping of related parameters in config.ini and in manual files. E.g. in this case the parameter is for the producer-plugin, so the parameter name starts with "producer-xyz".]

@bhazzard
Copy link

After discussion on the Node Operators call:

  • Name the option produce-block-offset-ms
  • Set the new default value to 450

heifner added a commit that referenced this issue Oct 19, 2023
heifner added a commit that referenced this issue Oct 20, 2023
heifner added a commit that referenced this issue Oct 20, 2023
heifner added a commit that referenced this issue Oct 20, 2023
heifner added a commit that referenced this issue Oct 20, 2023
heifner added a commit that referenced this issue Oct 20, 2023
heifner added a commit that referenced this issue Oct 21, 2023
[5.0] Replaced `cpu-effort-percent` with `produce-block-offset-ms`
heifner added a commit that referenced this issue Oct 23, 2023
[5.0 -> main] Replaced `cpu-effort-percent` with `produce-block-offset-ms`
@github-project-automation github-project-automation bot moved this from In Progress to Done in Team Backlog Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment