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

feat(zebra-db): Split low level database code inside zebra-state #7926

Open
Tracked by #7728
oxarbitrage opened this issue Nov 8, 2023 · 4 comments
Open
Tracked by #7728
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions A-state Area: State / database changes C-enhancement Category: This is an improvement

Comments

@oxarbitrage
Copy link
Contributor

oxarbitrage commented Nov 8, 2023

Motivation

We want to re-using RocksDB for our scanner work, because we already have well-tested low level interfaces to it.

Suggested Changes

We want to move low-level database code into:

And move anything specific to zebra-state outside those modules:

  • the Zebra-specific parts of upgrade.rs
  • anything specific to the column families, validation, or custom tasks in zebra-state

Related Work

This was mentioned in #7904 (comment) and earlier comments on that ticket.

@oxarbitrage oxarbitrage added P-Low ❄️ A-state Area: State / database changes A-blockchain-scanner Area: Blockchain scanner of shielded transactions labels Nov 8, 2023
@teor2345 teor2345 added the C-enhancement Category: This is an improvement label Nov 8, 2023
@teor2345
Copy link
Collaborator

teor2345 commented Nov 8, 2023

@arya2 this ticket is related to some ideas you had about making format upgrades generic.

The simplest change we could do is to pass a format upgrade function and a format check function to the upgrade task in the split out crate.

But how much more work would it be to refactor each upgrade into an upgrade trait, and then pass a list of upgrade trait objects to the upgrade task?

I guess we could do that later, it's not required to do the split.

@arya2
Copy link
Contributor

arya2 commented Nov 9, 2023

But how much more work would it be to refactor each upgrade into an upgrade trait, and then pass a list of upgrade trait objects to the upgrade task?

I don't think it would be too significant, but it would involve a lot of code movement that could be difficult to review alongside this change.

I guess we could do that later, it's not required to do the split.

It would be good to do it before the next format upgrade, I opened #7932

@teor2345
Copy link
Collaborator

teor2345 commented Nov 9, 2023

But how much more work would it be to refactor each upgrade into an upgrade trait, and then pass a list of upgrade trait objects to the upgrade task?

I don't think it would be too significant, but it would involve a lot of code movement that could be difficult to review alongside this change.

I was expecting this change to happen in 2-3 PRs:

  • optional: change upgrades into a trait
  • move zebra-state specific code out of the files that are going to be moved to zebra-db
  • create the zebra-db crate

@mpguerra
Copy link
Contributor

But how much more work would it be to refactor each upgrade into an upgrade trait, and then pass a list of upgrade trait objects to the upgrade task?

I don't think it would be too significant, but it would involve a lot of code movement that could be difficult to review alongside this change.

I was expecting this change to happen in 2-3 PRs:

* optional: change upgrades into a trait

This is #7932

* move zebra-state specific code out of the files that are going to be moved to zebra-db

This can be this issue (#7926)

* create the zebra-db crate

I've split this out into another checklist item in #7728

@teor2345 teor2345 changed the title feat(zebra-db): Split a zebra-db crate out of zebra-state feat(zebra-db): Split low level database code inside zebra-state Nov 13, 2023
@teor2345 teor2345 assigned teor2345 and unassigned teor2345 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions A-state Area: State / database changes C-enhancement Category: This is an improvement
Projects
Status: New
Development

No branches or pull requests

4 participants