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

Cache computation of TICKF rule #4054

Closed
dnadales opened this issue Oct 4, 2022 · 6 comments
Closed

Cache computation of TICKF rule #4054

dnadales opened this issue Oct 4, 2022 · 6 comments
Assignees

Comments

@dnadales
Copy link
Member

dnadales commented Oct 4, 2022

At the moment TICKF rule takes a considerable amount of time when the slot in question crosses the epoch boundary. This is especially problematic for the leader check, which gets called on every slot. The ledger view computed by TICKF is not cached so every leader check past the epoch boundary results in re-doing the computation.

This could alleviate problems like the one witnessed in this ticket.

Note that this is a short-term fix that immediately alleviates the performance problems observed in mainnet, but ultimately we should determine if it is worth the effort to make TICKF more efficient so no application is expensive.

This issue is tracking the determination of the costs of the TICKF rule.

@nfrisby
Copy link
Contributor

nfrisby commented Oct 19, 2022

This PR has a db-analyser branch with relevant measurements #4014

@nfrisby
Copy link
Contributor

nfrisby commented Oct 19, 2022

I haven't fully caught-up since vacation, but it sounds like any prompt fix here would happen in Consensus. If that's what happens, I think the workaround can be fully isolated in the leadership check loop in NodeKernel.hs.

Something like: we maintain state in that loop by ticking the header state along (resetting it whenever the local tip changes). Note that we're ticking it instead of forecasting. Ticking is a semigroup (forecasting is not), so we'll just need to tick from the last slot we checked to the next slot we're checking. And that means we'd only cross the epoch boundary once. That means we'll still miss checking several slots at the beginning of the epoch (approximately one slot per second of computation necessary to tick across the boundary), but after that behavior will be normal. It's a fixed number of slots of delay instead of waiting until the first block (luckily) gets forged.

@JaredCorduan
Copy link
Contributor

thanks to a clever idea from @nfrisby, we think we now have a stop-gap in the ledger. IntersectMBO/cardano-ledger#3141

@jasagredo
Copy link
Contributor

This seems like it can be closed because this other PR was merged in Ledger. Could you confirm? @JaredCorduan @nfrisby

@JaredCorduan
Copy link
Contributor

I agree @jasagredo , the ledger PR that you linked to achieves the same goal, but does so in the ledger instead of consensus. thanks for asking!

@jasagredo
Copy link
Contributor

Closing then. Thanks!

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

No branches or pull requests

4 participants