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

Provide low-level API for db-sync #88

Closed
jasagredo opened this issue Apr 20, 2023 · 2 comments · Fixed by #87
Closed

Provide low-level API for db-sync #88

jasagredo opened this issue Apr 20, 2023 · 2 comments · Fixed by #87
Assignees
Labels

Comments

@jasagredo
Copy link
Contributor

The integration work of the first version of UTxO-HD with db-sync was intended to avoid using tables and just carrying an EmptyMK ledger state all over the execution, with the full UTxO set inside, just as before UTxO-HD.

However, it turns out that our API is too restrictive as it requires ValuesMK for ticking and applying blocks, which will then be stowed. It is perfectly fine for Consensus' concerns but not for this db-sync integration as stowing/unstowing the full UTxO set duplicates the map on each step and therefore memory consumption explodes.

We should provide lower level alternatives to the tickThenReapply function that don't expect to have the tables unstowed, i.e.

tickThenReapply' :: ... -> LedgerState blk EmptyMK -> LedgerState blk EmptyMK

Possibly this just means factoring out some of the things we do (the stowing and unstowing of values) into a separate function, something like tickThenReapplyWithTables or some such, to get:

tickThenReapplyWithTables :: ... -> LedgerState blk ValuesMK -> LedgerState blk ValuesMK
tickThenReapplyWithTables = unstow . tickThenReapply' . stow

(Probably with some applyDiffs in between).

db-sync has a half-working integration which uses ouroboros-network at ea8f657533b64ad3494d3695c945c8fb24c6a0f1.

I think we should just start a branch there and do whatever we need to do to provide this API, but even maybe only in the packages needed by db-sync, to not get into trouble fighting the tests, as this is exploratory integration work. In particular db-sync uses:

  • ouroboros-consensus
  • ouroboros-consensus-diffusion
  • ouroboros-consensus-protocol
  • ouroboros-consensus-byron
  • ouroboros-consensus-shelley
  • ouroboros-consensus-cardano
@jorisdral jorisdral self-assigned this Apr 20, 2023
@kderme
Copy link
Contributor

kderme commented Apr 24, 2023

Thanks! Fyi this was my attempt to use the existing api https://github.com/input-output-hk/cardano-db-sync/blob/e841d95123d25f054ef355a5734bc123bb6dcae1/cardano-db-sync/src/Cardano/DbSync/LedgerState/ConsensusApi.hs#L30. It had performance issues, because of the need to unstow and stow back the ledger state.

@jorisdral jorisdral transferred this issue from IntersectMBO/ouroboros-network May 16, 2023
@jorisdral jorisdral linked a pull request May 16, 2023 that will close this issue
@jorisdral jorisdral moved this from 🏗 In progress to 👀 In review in Consensus Team Backlog May 22, 2023
@dnadales
Copy link
Member

dnadales commented Jun 5, 2023

This depends on #99

@jorisdral jorisdral moved this from 👀 In review to 🚫 Help needed in Consensus Team Backlog Jun 13, 2023
@dnadales dnadales moved this from 🚫 Help needed to 🏗 In progress in Consensus Team Backlog Jun 26, 2023
@jorisdral jorisdral moved this from 🏗 In progress to 👀 In review in Consensus Team Backlog Jun 27, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Consensus Team Backlog Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants