-
Notifications
You must be signed in to change notification settings - Fork 790
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
Introduce active_transactions_config
#4604
Introduce active_transactions_config
#4604
Conversation
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.
Nice cleanup. The question is whether we want to call this config [nano.active_transactions]
to keep consistency with class naming (transaction name is a bit unfortunate in my opinion) or [nano.active_elections]
to keep consistency with previous config naming.
I asked myself the same question. |
Renaming the class to |
I renamed
And it can be used in the [node.active_elections]
size = 5000
hinted_limit_percentage = 20
optimistic_limit_percentage = 10
confirmation_history_size = 2048
confirmation_cache = 65536 |
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.
LGTM. Nice cleanup and agree on the class rename.
- remove old config variables
- active_elections_size - active_elections_hinted_limit_percentage - active_elections_optimistic_limit_percentage - confirmation_history_size
4b0434d
to
4a00f05
Compare
Introduce
active_transactions_config
:Newly introduced
confirmation_cache
is used inrecently_confirmed{ config.confirmation_cache },
instead of the hardcoded value65536