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

Updates Conan dependencies: RocksDB #5335

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open

Updates Conan dependencies: RocksDB #5335

wants to merge 27 commits into from

Conversation

bthomee
Copy link
Collaborator

@bthomee bthomee commented Mar 6, 2025

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:

  • The latest version of RocksDB is currently 9.11.1; however, Conan 1.x only supports up to 9.7.3.
  • Once we switch to Conan 2.x we can use RocksDB versions 9.7.4 and 9.10.0 without needing to patch it, as the recipe already applies the patch to those two versions.
  • As 9.7.4 fixes a memory leak, I also included that patch in this PR.

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

  • Refactor (non-breaking change that only restructures code)

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:

[node_db]
type=RocksDB
path=tmp/rocksdb

and started rippled without existing database, and it synced fine with testnet as a regular (non-validator) node in about 30mins. I used curl 127.0.0.1:51234 -sd '{"method": "server_info"}' | jq -r ".result.info | {hostid, server_state, complete_ledgers, git, peers, uptime}" periodically until the server_state showed full with a range of complete_ledgers.

2. Upgrade

I used a rippled.cfg containing the recommended configuration as once, a long time ago, stated in the release notes:

[node_db]
type=RocksDB
path=tmp/rocksdb
open_files=1200
filter_bits=12
cache_mb=256
file_size_mb=8
file_size_mult=2
fast_load=1

I used the latest code from the develop branch, which uses RocksDB 6.29.5, with no existing database, and after rerunning conan install and the CMake commands I then let rippled 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:

Copy link

codecov bot commented Mar 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 78.1%. Comparing base (2216e5a) to head (899a39c).

Files with missing lines Patch % Lines
src/xrpld/nodestore/backend/RocksDBFactory.cpp 0.0% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Files with missing lines Coverage Δ
src/xrpld/nodestore/backend/RocksDBFactory.cpp 53.7% <0.0%> (-0.5%) ⬇️

... and 3 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bthomee bthomee requested review from legleux and vvysokikh1 March 9, 2025 14:55
@bthomee bthomee marked this pull request as ready for review March 9, 2025 14:55
@Bronek Bronek added the Perf Test Desired (Optional) RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish label Mar 10, 2025
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants