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

Create internal/database/badger package and drop chaindb #2982

Closed
qdm12 opened this issue Nov 29, 2022 · 1 comment
Closed

Create internal/database/badger package and drop chaindb #2982

qdm12 opened this issue Nov 29, 2022 · 1 comment
Assignees

Comments

@qdm12
Copy link
Contributor

qdm12 commented Nov 29, 2022

A few key points why this is needed:

  • ⚠️ With fix(trie): remove encoding buffers pool #2929 and recent commits, the in-memory tries are no longer leaking memory, and it can clearly be seen our current database implementation leaks memory pretty bad and also uses 50% of our CPU usage, which is also bad.
  • having a separate repository for the database implementation (and interfaces) doesn't make sense now and probably won't in the future either.
  • Interfaces, constructors are oddly designed in the chaindb repository
  • chaindb uses a rather outdated and obscure fork of badgerdb

Prioritizing this issue right now, especially to see the result memory-usage-wise in order to take the right decision regarding state code changes (lazy loading, v1 state trie, other performance considerations).

The plan is to create an internal/database/badger package using badger/v3 with cleaner constructors and interfaces in the rest of the code, and remove the chaindb dependency. A very simple internal/database/memory package might be created as well to ensure we have the right interfaces for multiple implementations. More database implementations can be added later on if needed for different memory or CPU usage trade offs.

This is blocked by #2981 since our code is rather tightly coupled unfortunately when it comes to replacing the database.

@dimartiro
Copy link
Contributor

I think we can close this issue since we replaced chaindb to pebble some time ago. What do you think @timwu20 ?

@P1sar P1sar removed state labels Jan 15, 2024
@timwu20 timwu20 closed this as completed Jan 15, 2024
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

5 participants