feat(storage): CometBFT no longer uses cometbft-db #4514
feat(storage): CometBFT no longer uses cometbft-db #4514
Conversation
Partially addresses #4486. ### Context CometBFT will stop importing [cometbft-db](https://github.com/cometbft/cometbft-db) as a dependency to support multiple database backends. Instead, it will use [pebble](https://github.com/cockroachdb/pebble) by default. ### Changes This PR adds package `storage` to the `internal` packages: - it exports the `DB`, `Batch`, and `Iterator` interfaces that database backends must implement to work with CometBFT. Previously, they were offered by cometbft-db. - it exports the pebbledb implementation of the above interfaces. Users can provide their own implementation of these interfaces using a different database under the hood. However, we will support and maintain only the pebble implementation that CometBFT will use. **Note**: this PR ports the code currently provided by cometbft-db. It does not add new functionalities to either the interfaces or their implementation using pebble. What it does: - improve docs - clearer error messages - minor refactoring of nested `if` statements for readability --- #### PR checklist - ~[] Tests written/updated~ - ~[ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments
|
@alesforz the PR says |
…face. (#4515) Partially addresses #4486. ### Context CometBFT will stop importing [cometbft-db](https://github.com/cometbft/cometbft-db) as a dependency to support multiple database backends. Instead, it will use [pebble](https://github.com/cockroachdb/pebble) by default. ### Changes This PR adds unit tests to the pebble implementation of the `DB` interface that CometBFT will use once we remove cometbft-db.
What I mean is that I will work on smaller, separate PRs to implement the TODO list, so that they are easier to review and (hopefully) approved. Ideally, the code in this PR doesn't need review, because all of it has been already reviewed and approved in the smaller PRs. |
…terface. (#4522) Partially addresses #4486. ### Context CometBFT will stop importing [cometbft-db](https://github.com/cometbft/cometbft-db) as a dependency to support multiple database backends. Instead, it will use [pebble](https://github.com/cockroachdb/pebble) by default. ### Changes This PR adds unit tests to the pebble implementation of the `Batch` interface that CometBFT will use once we remove cometbft-db.
…to_feature_remove-cometbftdb
… interface. (#4530) Partially addresses #4486. ### Context CometBFT will stop importing [cometbft-db](https://github.com/cometbft/cometbft-db) as a dependency to support multiple database backends. Instead, it will use [pebble](https://github.com/cockroachdb/pebble) by default. ### Changes This PR adds unit tests to the pebble implementation of the `Iterator` interface that CometBFT will use once we remove cometbft-db.
…to_feature_remove-cometbftdb
…to_feature_remove-cometbftdb
Partially addresses #4486. ### Context CometBFT will stop importing [cometbft-db](https://github.com/cometbft/cometbft-db) as a dependency to support multiple database backends. Instead, it will use [pebble](https://github.com/cockroachdb/pebble) by default. ### Changes This PR adds type `PrefixDB` to `internal/storage` from cometbft-db. `PrefixDB` is a database wrapper that implements the`DB`, `Batch`, and `Iterator` interfaces. **Note**: this PR makes some changes to the implementation of `PrefixDB`. Namely: - `PrefixDB` no longer uses a mutex, because we think that it is sufficient to rely on the wrapped database's concurrency control mechanism. That is, concurrent calls to `PrefixDB`'s methods will already "lock" on the calls to the wrapped DB methods. - `NewPrefixDB` now returns an error if the given prefix is empty. We think allowing the creation of a `PrefixDB` with no prefix was an oversight in the design. - `prefixDBBatch` methods now want a pointer receiver. - `PrefixDB.Compact` now prepends the prefix to `start` and `end` keys defining the iterator's range. The previous implementation did not do so, and this was likely an oversight. We believed this behavior contradicted the definition of a `PrefixDB` as a logical database that prepends the prefix to all keys before operating on the underlying database.
…in_into_feature_remove-cometbftdb
|
can this be closed in favor of #4601? |
Partially addresses #4486. This PR removes cometbft-db from cometbft build process: - Dockerfiles - Makefiles - CI
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This feature branch stores the work we are doing to close #1039.
We will merge related PRs to this branch rather than
main.Context
CometBFT will stop importing
cometbft-db as a dependency to support multiple database backends. Instead, it will use pebble by default.
Changes merged so far
storagein theinternalpackages (PR):DB,Batch, andIteratorinterfaces that were previously offered by cometbft-db.DBinterface using pebble (PR).Batchinterface using pebble (PR).Iteratorinterface using pebble (PR).PrefixDBand its implementation of theDB,Batch, andIteratorinterfaces (PR).Note for Readers
All the code in this PR has already been reviewed and approved in the smaller PRs linked above. All commits are either merge commits of those PRs or merges from the
mainbranch to keep this branch up to date.Therefore, to review the changes, I suggest looking at the list above and going to the specific PR that implemented the changes you're interested in. This should be easier than trying to review all the changes at once in this PR.
PR checklist
[ ] Tests written/updated.changelog(we use unclog to manage our changelog)[ ] Updated relevant documentation (docs/orspec/) and code comments