-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Updates Conan dependencies: RocksDB #5335
Open
bthomee
wants to merge
27
commits into
develop
Choose a base branch
from
bthomee/deps3
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+378
−89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5335 +/- ##
=========================================
- Coverage 78.1% 78.1% -0.0%
=========================================
Files 790 790
Lines 67908 67910 +2
Branches 8230 8228 -2
=========================================
- Hits 53034 53033 -1
- Misses 14874 14877 +3
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Perf Test Desired (Optional)
RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
This PR updates RocksDB to version 9.7.3. Only minor changes were necessary to the code to upgrade from version 6.29.5. The updated conandata.yml and conanfile.py were copied from the RocksDB repository, and the updated patch was obtained from a merged PR in the Conan Center Index.
Notes:
Extra changes included in this PR are (i) disabling the upload of updated packages to the internal Conan proxy/registry, since that will always fail due to the GitHub runner not having access to the username/password, and (ii) removing the
DBOOST_ASIO_DISABLE_CONCEPTS
cxxflag as the issue described in BUILD.md does not affect our MacOS runner & actually results in that flag being added 100s of times.Context of Change
Dependency scanning revealed that current dependencies are out of date. Updating dependencies allows us to take advantage of bug fixes, new features, and/or security improvements.
Type of Change
Test Plan
I performed the following two tests.
1. Clean
Run
conan install
using the code in this branch, configure CMake, and then build with CMake. The output showed that RocksDB 9.7.3 was used.I used a basic
rippled.cfg
:and started
rippled
without existing database, and it synced fine with testnet as a regular (non-validator) node in about 30mins. I usedcurl 127.0.0.1:51234 -sd '{"method": "server_info"}' | jq -r ".result.info | {hostid, server_state, complete_ledgers, git, peers, uptime}"
periodically until theserver_state
showed full with a range ofcomplete_ledgers
.2. Upgrade
I used a
rippled.cfg
containing the recommended configuration as once, a long time ago, stated in the release notes:I used the latest code from the
develop
branch, which uses RocksDB 6.29.5, with no existing database, and after rerunningconan install
and the CMake commands I then letrippled
run for a while. After stopping the binary, I reinitialized all but now with RocksDB 9.7.3. The binary was able to resume using the older database, suggesting no incompatibility issues. The RocksDB page also indicates newer versions will be backward compatible.The curl command above provided a sanity check that I was also working on the correct branch. See also the attached log files: