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

Configurable BlockFetch parameters #2363

Merged
merged 1 commit into from
Jul 9, 2020
Merged

Configurable BlockFetch parameters #2363

merged 1 commit into from
Jul 9, 2020

Conversation

karknu
Copy link
Contributor

@karknu karknu commented Jul 7, 2020

No description provided.

ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
@mrBliss mrBliss changed the title Configurable Blockfatch parameters Configurable BlockFetch parameters Jul 7, 2020
@karknu karknu requested a review from coot July 7, 2020 07:49
@@ -145,6 +146,12 @@ data RunNodeArgs blk = RunNodeArgs {
--
-- Use 'defaultClockSkew' when unsure.
, rnMaxClockSkew :: ClockSkew

Copy link
Contributor

Choose a reason for hiding this comment

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

The principle I originally had in mind when we introduced RunNodeArgs is to use these fields for parameters for which we can't pick default values, e.g., NetworkMagic, ProtocolInfo, etc. For NodeArgs and ChainDbArgs, we can, so we provide functions to customise them without having to force the user (i.e., cardano-node) to pick the defaults (we know better which defaults to pick).

Over the time RunNodeArgs has been changed by multiple people and the principle no longer holds. (I want to refactor/clean up this mess at some point).

That's the reason why I prefer a "customise" function instead of requiring concrete values.

Enforce some invariants with `nodeArgsEnforceInvariants`.
@mrBliss
Copy link
Contributor

mrBliss commented Jul 9, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 9, 2020

@iohk-bors iohk-bors bot merged commit fc54848 into master Jul 9, 2020
@iohk-bors iohk-bors bot deleted the karknu/block_cfg branch July 9, 2020 16:37
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