Support for big ledger peer snapshot in the topology file #5787
+395
−131
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
crocodile-dentist
force-pushed
the
mwojtowicz/genesis-ledger-peer-snapshot
branch
from
b281d8f
to
7a66082
Compare
carbolymer
reviewed
Apr 18, 2024
cardano-node/src/Cardano/Node/Run.hs
Outdated
Comment on lines
796
to
797
| (\io_m_lps -> do | ||
| m_lps <- io_m_lps |
There was a problem hiding this comment.
I though io_m_lps was a little clearer than ioMLps, or do you have something else in mind?
There was a problem hiding this comment.
Having to prefix the variables with the types is usually a sign that you're working against the language. Here it is because you're trying to shoehorn your function into MaybeT which doesn't work here very well and forces you to work with variables with type IO (Maybe a) outside of MaybeT. Why not write this function like this:
updateLedgerPeerSnapshot :: Tracer IO (StartupTrace blk)
-> STM IO (Maybe PeerSnapshotFile)
-> (Maybe LedgerPeerSnapshot -> STM IO ())
-> IO (Maybe LedgerPeerSnapshot)
updateLedgerPeerSnapshot startupTracer readLedgerPeerPath writeVar = do
mPeerSnapshotFile <- atomically readLedgerPeerPath
mLedgerPeerSnapshot <- forM mPeerSnapshotFile $ \f -> do
lps@(LedgerPeerSnapshot (wOrigin, _)) <- readPeerSnapshotFile f
lps <$ traceWith startupTracer (LedgerPeerSnapshotLoaded wOrigin)
atomically . writeVar $ mLedgerPeerSnapshot
pure mLedgerPeerSnapshotThere was a problem hiding this comment.
Yes, that's most intuitive.
carbolymer
approved these changes
Apr 18, 2024
|
Maybe I don't have to bump the protocol version at all in this patch based on review comment in related PR on consensus side |
crocodile-dentist
force-pushed
the
mwojtowicz/genesis-ledger-peer-snapshot
branch
from
7a66082
to
c10e12e
Compare
|
This PR is stale because it has been open 45 days with no activity. |
crocodile-dentist
force-pushed
the
mwojtowicz/genesis-ledger-peer-snapshot
branch
from
c10e12e
to
6b9616c
Compare
crocodile-dentist
force-pushed
the
mwojtowicz/genesis-ledger-peer-snapshot
branch
from
6b9616c
to
70b3121
Compare
coot
changed the title
This change introduces support for big ledger peers in the node. A new optional entry in network topology JSON parser is added that is intended to point to a path containing a serialized snapshot of big ledger peers taken from some slot a priori. When present, this file is decoded at node startup, or when a SIGHUP is triggered, and made available to the diffusion layer via reading from a TVar.
The ConsensusMode configuration option determines whether support for Genesis syncing is enabled. If this flag is GenesisMode, alternate targets for active peers (non- big ledger) and all big ledger targets is used by the peer selection and churn governors in the diffusion layer. These targets are in effect when ledger state is deemed `TooOld` by consensus layer. The large active big ledger peer targets crucially depend on chain skipping to maintain performance requirements. Otherwise, in PraosMode, the legacy algorithm with a single set of targets is employed.
Missing updating Node Configuration parser to read genesis targets and ledger snapshot.
crocodile-dentist
force-pushed
the
mwojtowicz/genesis-ledger-peer-snapshot
branch
from
c427e76
to
982cd1d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change introduces support for big ledger peers in the node. A new optional entry in network topology JSON parser is added that is intended to point to a path containing a serialized snapshot of big ledger peers taken from some slot a priori. When present, this file is decoded at node startup, or when a SIGHUP is triggered, and made available to the diffusion layer via reading from a TVar.
Checklist
See Runnings tests for more details
CHANGELOG.mdfor affected package.cabalfiles are updatedhlint. See.github/workflows/check-hlint.ymlto get thehlintversionstylish-haskell. See.github/workflows/stylish-haskell.ymlto get thestylish-haskellversionghc-8.10.7andghc-9.2.7Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.