-
Notifications
You must be signed in to change notification settings - Fork 14
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
refactor(db)!: remove support for cleveldb, boltdb, rocksdb, badgerdb #974
Conversation
WalkthroughThe pull request modifies multiple files to streamline the Tendermint build and configuration process by removing support for several database backends, specifically Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)config/config.go (2)
The comment changes accurately reflect:
The validation implementation:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
test/e2e/networks/dashcore.toml (1)
Line range hint
1-4
: Consider documenting test coverage strategyWhile the configuration maintains comprehensive test coverage across different scenarios (ABCI protocols, sync modes, perturbations), consider adding a comment block explaining the test coverage strategy. This would help maintainers understand the purpose of each node's configuration.
# This testnet is run by CI, and attempts to cover a broad range of # functionality with a single network. +# +# Test coverage matrix: +# - validator02: TCP ABCI + immediate persistence +# - validator03: Block retention + periodic persistence + kill recovery +# - validator04: Builtin ABCI + pause recovery +# - validator05: Late joining + block sync + state sync + multiple perturbationstest/e2e/networks/rotate.toml (1)
123-146
: Consider updating test documentation.Since this test configuration previously validated multiple database backends, it would be helpful to update the test documentation to clarify that database backend variation is no longer part of the test coverage.
Would you like me to help draft documentation updates to reflect these changes?
docs/nodes/configuration.md (3)
51-51
: Enhance database backend documentation.While the documentation correctly lists goleveldb as the supported backend, consider adding:
- An explicit note that other backends (cleveldb, boltdb, rocksdb, badgerdb) are no longer supported
- Migration instructions for users currently using the removed backends
-# Database backend: goleveldb -# * goleveldb (github.com/syndtr/goleveldb - most popular implementation) -# - pure go -# - stable +# Database backend: goleveldb (only supported option) +# * goleveldb (github.com/syndtr/goleveldb - most popular implementation) +# - pure go +# - stable +# +# Note: Support for cleveldb, boltdb, rocksdb, and badgerdb has been removed. +# If you were using any of these backends, please migrate your data to goleveldb. +# Migration guide: [TODO: Add link to migration guide]
535-537
: Fix formatting inconsistency in new parameters section.The formatting of the new parameters section is inconsistent with the rest of the document.
-# There are three new parameters, which are enabled if use-legacy is set to false. - -# queue-type = sets a type of queue to use in the p2p layer. There are three options available `fifo`, `priority` and `wdrr`. The default is priority -# bootstrap-peers = is a list of comma seperated peers which will be used to bootstrap the address book. -# max-connections = is the max amount of allowed inbound and outbound connections. +### New Parameters + +These parameters are enabled when `use-legacy` is set to false: + +- `queue-type` = sets the type of queue to use in the p2p layer. Available options: `fifo`, `priority`, and `wdrr`. Default: `priority` +- `bootstrap-peers` = a comma-separated list of peers used to bootstrap the address book +- `max-connections` = maximum number of allowed inbound and outbound connections
580-580
: Enhance PostgreSQL schema setup documentation.The PostgreSQL schema setup command could be more detailed and properly formatted.
-psql ... -f state/indexer/sink/psql/schema.sql +# Apply the PostgreSQL schema (replace the connection parameters with your values) +psql -h <hostname> -p <port> -U <username> -d <database> -f state/indexer/sink/psql/schema.sql
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
Makefile
(0 hunks)cmd/tenderdash/commands/inspect.go
(0 hunks)cmd/tenderdash/commands/run_node.go
(0 hunks)config/config.go
(2 hunks)config/toml.go
(1 hunks)docs/nodes/configuration.md
(3 hunks)test/e2e/Makefile
(1 hunks)test/e2e/README.md
(2 hunks)test/e2e/generator/generate.go
(0 hunks)test/e2e/networks/ci.toml
(3 hunks)test/e2e/networks/dashcore.toml
(2 hunks)test/e2e/networks/island.toml
(2 hunks)test/e2e/networks/rotate.toml
(1 hunks)test/e2e/pkg/manifest.go
(1 hunks)test/e2e/pkg/testnet.go
(1 hunks)
💤 Files with no reviewable changes (4)
- Makefile
- cmd/tenderdash/commands/inspect.go
- cmd/tenderdash/commands/run_node.go
- test/e2e/generator/generate.go
✅ Files skipped from review due to trivial changes (1)
- test/e2e/README.md
🔇 Additional comments (15)
test/e2e/networks/island.toml (3)
2-2
: LGTM: Formatting improvement
The formatting change improves readability while maintaining the same functionality.
52-52
: Consistent database backend configuration across all validators
All validator nodes have been properly updated to use "goleveldb" as their database backend, maintaining consistency across the test network configuration.
Also applies to: 59-59, 66-66, 72-72, 78-78
45-45
: Database backend standardization looks good
The change to "goleveldb" aligns with the PR objective to standardize database backends.
Let's verify there are no remaining references to other database backends in the e2e test configurations:
✅ Verification successful
Database backend standardization verified
All database configurations in the e2e test files have been standardized to use "goleveldb". The verification found:
- All explicit database configurations in network TOML files (island.toml, ci.toml, rotate.toml, dashcore.toml) use "goleveldb"
- No other database backend references were found in the remaining e2e test configuration files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to removed database backends in e2e test configs
echo "Checking for removed database backends in e2e test configs..."
rg -g '*.toml' "database.*=.*(cleveldb|boltdb|rocksdb|badgerdb)" test/e2e/
Length of output: 209
Script:
#!/bin/bash
# Let's first check all database configurations in e2e test configs
echo "Checking all database configurations in e2e test configs..."
rg -g '*.toml' "database.*=.*" test/e2e/
# Also check if there are any other TOML files that might contain database configs
echo -e "\nListing all TOML files in test/e2e/..."
fd -e toml . test/e2e/
Length of output: 1516
Script:
#!/bin/bash
# Let's check the remaining TOML files that didn't show up in the database search
echo "Checking database configurations in simple.toml, single.toml, built-in.toml, and socket.toml..."
rg "database" test/e2e/networks/simple.toml test/e2e/networks/single.toml test/e2e/node/built-in.toml test/e2e/node/socket.toml
Length of output: 328
test/e2e/Makefile (1)
29-29
: LGTM! Build tag simplification aligns with PR objectives.
The removal of database-related build tags (badgerdb,boltdb,cleveldb
) while retaining deadlock
is consistent with the PR's goal of removing support for unused database backends.
Let's verify that no other database-related build tags exist in the codebase:
✅ Verification successful
Build tag removal is safe and consistent with codebase changes
The verification confirms that:
- The database-related config files are test fixtures in
internal/libs/confix/testdata/
, as evidenced by their location and the associated test fileconfix_test.go
. - No database backend implementations remain in the codebase.
- No imports of the removed database packages exist.
- The removal of database-related build tags from the Makefile aligns with the complete removal of these database backends.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining database-related build tags
# Expected: No occurrences of removed database build tags
echo "Checking for database-related build tags in go files..."
rg -l "//\s*\+build.*(?:badger|bolt|clevel|rock)db"
echo "Checking for database-related tags in build commands..."
rg -l "\-tags.*(?:badger|bolt|clevel|rock)db"
Length of output: 619
Script:
#!/bin/bash
# Let's check the content of these config files to understand if they are just test fixtures
# or actual configuration files that need updating
echo "Checking content of config files:"
for file in internal/libs/confix/testdata/v*-config.toml; do
echo "=== $file ==="
grep -A 5 -B 5 "tags" "$file"
done
# Also verify if there are any other references to these databases in the codebase
echo -e "\nChecking for any other database references:"
rg -l "(?i)(badger|bolt|clevel|rock)db" --type go
Length of output: 12950
Script:
#!/bin/bash
# Let's verify if these are just test fixtures by checking their location and usage
echo "Checking if these are test fixtures:"
rg -l "testdata" --type go internal/libs/confix/
# Also check if there are any actual database backend implementations remaining
echo -e "\nChecking for database implementation files:"
fd -e go -x grep -l "type.*DB struct" {}
# Check for any remaining database imports
echo -e "\nChecking for database-related imports:"
rg -l '(?i)import.*".*/(badger|bolt|levigo|rocksdb)"'
Length of output: 464
test/e2e/networks/ci.toml (1)
44-44
: Database backend standardization looks good!
The changes consistently update all nodes to use goleveldb
as the database backend, which aligns with the PR objective to remove support for alternative database implementations.
Let's verify that no old database backends remain in the configuration:
Also applies to: 52-52, 62-62, 70-70, 79-79
✅ Verification successful
Database backend standardization successfully verified
All database configurations across the test networks (ci.toml, island.toml, rotate.toml, dashcore.toml) have been standardized to use "goleveldb". No instances of removed database backends (cleveldb, boltdb, rocksdb, badgerdb) were found in any TOML configuration files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to removed database backends
# Expected: No matches should be found
echo "Checking for removed database backends in TOML files..."
rg -i "database.*=.*\"(cleveldb|boltdb|rocksdb|badgerdb)\"" --type toml
Length of output: 192
Script:
#!/bin/bash
# Let's verify all database configurations in TOML files
# First, find all database configurations
echo "All database configurations in TOML files:"
rg -i "database.*=.*\".*\"" --type toml
# Also check the specific file mentioned in the review
echo -e "\nSpecific file content:"
cat test/e2e/networks/ci.toml
Length of output: 2995
test/e2e/networks/dashcore.toml (4)
59-59
: LGTM: Database change aligns with standardization effort
The change to goleveldb for validator02 maintains test coverage for TCP ABCI protocol scenarios with immediate persistence.
67-67
: LGTM: Maintains critical test coverage
The switch to goleveldb for validator03 preserves important test scenarios including block retention, periodic persistence, and kill recovery behavior.
77-77
: LGTM: Preserves builtin ABCI protocol testing
The database change for validator04 maintains test coverage for goleveldb with the builtin ABCI protocol implementation.
85-85
: LGTM: Maintains comprehensive test scenarios
The change to goleveldb for validator05 preserves critical test coverage for late-joining validators, block sync, state sync, and multiple failure modes.
test/e2e/networks/rotate.toml (1)
123-123
: Database backend changes align with PR objectives.
The changes consistently update all validator nodes to use goleveldb
, removing the previous test coverage for boltdb
, badgerdb
, and cleveldb
backends. This aligns with the PR's goal of standardizing on goleveldb
.
Let's verify there are no remaining references to the removed database backends in the test configurations:
Also applies to: 131-131, 139-139, 146-146
✅ Verification successful
The previous search might have failed because the files were not present or the pattern was too specific. Let's try a broader search to see all database configurations in TOML files to verify the migration is complete.
Database backend migration successfully verified
All database configurations in test TOML files (rotate.toml
, island.toml
, dashcore.toml
, and ci.toml
) consistently use "goleveldb" as the backend, confirming complete migration away from other database backends.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to removed database backends in test configurations
rg -g '*.toml' "boltdb|badgerdb|cleveldb|rocksdb" test/
Length of output: 57
Script:
#!/bin/bash
# Let's verify all database configurations in TOML files to ensure complete migration
rg -g '*.toml' 'database\s*=\s*"[^"]+"' test/
Length of output: 1042
test/e2e/pkg/manifest.go (1)
135-136
: LGTM! Documentation updated to reflect database backend changes.
The documentation update correctly reflects that only "goleveldb" is now supported, which aligns with the PR objective to remove support for other database backends.
Let's verify that all E2E test configurations are using "goleveldb":
✅ Verification successful
All E2E test configurations are already using "goleveldb" as the database backend
The verification confirms that all database configurations in the E2E test files (test/e2e/networks/*.toml
) are consistently set to "goleveldb". This aligns perfectly with the documentation update in test/e2e/pkg/manifest.go
stating that only "goleveldb" is supported.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check all E2E test configurations to ensure they only use goleveldb
# Test: Search for any database configuration in test files. Expect: Only "goleveldb" or default values.
echo "Checking E2E test configurations for database settings..."
rg -g '*.toml' 'database.*=.*' test/e2e/
Length of output: 1162
test/e2e/pkg/testnet.go (2)
Line range hint 539-543
: LGTM! Database validation aligns with PR objectives.
The validation logic correctly restricts database options to only "goleveldb" and "memdb", which aligns with the PR's goal of removing support for cleveldb, boltdb, rocksdb, and badgerdb backends.
Line range hint 539-543
: Verify test configurations and documentation consistency.
Let's ensure there are no test configurations using the removed database backends and that documentation is updated accordingly.
config/toml.go (1)
104-107
: Documentation update accurately reflects database backend changes.
The simplified documentation correctly states that only goleveldb is supported, which aligns with the PR's objective to remove support for other database backends.
Let's verify that references to removed database backends have been cleaned up:
✅ Verification successful
Let me try a different search pattern to verify the database backend implementation:
Documentation update accurately reflects database backend changes
The verification confirms that:
- The code consistently enforces goleveldb as the only supported database backend through validation in
config/config.go
- All database-related code paths use goleveldb or memdb (for testing)
- No references to removed database backends (cleveldb, boltdb, rocksdb, badgerdb) remain in the codebase
- The documentation in
config/toml.go
correctly reflects the current state of the codebase
The changes are consistent across the entire codebase and properly documented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to removed database backends
echo "Searching for references to removed database backends..."
rg -i "cleveldb|boltdb|rocksdb|badgerdb" --type go
# Search for database backend validation logic
echo "Checking database backend validation..."
ast-grep --pattern 'if $x == "goleveldb" { $$$
} else { $$$ }'
Length of output: 329
Script:
#!/bin/bash
# Search for database backend related code
echo "Searching for database backend related code..."
rg -i "database.*backend|db.*backend" --type go -A 3
# Search for database type validation
echo "Checking database type validation..."
rg -i "dbtype|db_type|database.*type" --type go -A 3
# Search for goleveldb references
echo "Checking goleveldb references..."
rg -i "goleveldb" --type go -A 3
Length of output: 15258
config/config.go (1)
200-205
: LGTM! Clear documentation of supported database backend.
The comment accurately reflects that only goleveldb is supported while maintaining a note about memdb for testing purposes.
@coderabbitai review |
✅ Actions performedReview triggered.
|
Issue being fixed or feature implemented
Tenderdash uses only
goleveldb
database backend. However, we have support for other database backends, like cleveldb, boltdb, rocksdb, badgerdb, that are not used. One of these backends, badgerdb, causes issues in tests due to compilation error.In addition,only
goleveldb
is considered as stable by upstream project - https://github.com/cometbft/cometbft-db .What was done?
Dropped support for unused database engines: cleveldb, boltdb, rocksdb, badgerdb.
How Has This Been Tested?
GHA pipeline
Breaking Changes
Dropped support of cleveldb, boltdb, rocksdb, badgerdb (as far as we know, these are not used anyway).
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes
New Features
goleveldb
is now supported across various components.Documentation
goleveldb
.Bug Fixes