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

[feat] Add scripts for updating BK RocksDB ini files #23178

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Aug 15, 2024

Fixes #23176

Motivation

The way RocksDB is configured changed in Pulsar 2.11 / Bookkeeper 4.15 .
The settings are handled in conf/entry_location_rocksdb.conf file and dbStorage_rocksDB_* keys in bookkeeper.conf have no effect. This is mainly a problem for Kubernetes deployments since there's not an easy way to override the configs.

Modifications

  • add generics script update-ini-from-env.py for updating keys in RocksDB ini files by environment variables
  • add compatibility script update-rocksdb-conf-from-env.py for updating conf/entry_location_rocksdb.conf file with dbStorage_rocksDB_* keys in the environment.

Additional context

With these changes, it's possible to modify a Pulsar Helm chart's bookkeeper/autorecovery commands to include && (([ -x bin/update-rocksdb-conf-from-env.py ] && bin/update-rocksdb-conf-from-env.py) || true) after bin/apply-config-from-env.py conf/bookkeeper.conf

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.0.0 milestone Aug 15, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 15, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.58%. Comparing base (bbc6224) to head (e2af409).
Report is 532 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23178      +/-   ##
============================================
+ Coverage     73.57%   74.58%   +1.01%     
- Complexity    32624    33555     +931     
============================================
  Files          1877     1921      +44     
  Lines        139502   144480    +4978     
  Branches      15299    15809     +510     
============================================
+ Hits         102638   107761    +5123     
+ Misses        28908    28461     -447     
- Partials       7956     8258     +302     
Flag Coverage Δ
inttests 27.81% <ø> (+3.23%) ⬆️
systests 24.70% <ø> (+0.38%) ⬆️
unittests 73.93% <ø> (+1.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 501 files with indirect coverage changes

@lhotari lhotari merged commit 3ada566 into apache:master Aug 16, 2024
53 of 54 checks passed
lhotari added a commit that referenced this pull request Aug 16, 2024
lhotari added a commit that referenced this pull request Aug 16, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2024
(cherry picked from commit 3ada566)
(cherry picked from commit ca4512c)
@lhotari
Copy link
Member Author

lhotari commented Aug 16, 2024

One of the tunings that is lost in Pulsar 2.11 / Bookkeeper 4.15 is the automatic adjustment of dbStorage_rocksDB_blockCacheSize for the entry index location database

# Size of RocksDB block-cache. For best performance, this cache
# should be big enough to hold a significant portion of the index
# database which can reach ~2GB in some cases
# Default is to use 10% of the direct memory size
dbStorage_rocksDB_blockCacheSize=

since Pulsar 2.11 / Bookkeeper 4.15, this isn't automatically adjusted. the value is fixed to 206150041 which is less than 200MB.
This could have an impact when the value used to be larger in Pulsar 2.10 due to automatic tuning. The bottleneck would show up in the Bookkeeper's index lookup metrics which were added in BK 4.16 with apache/bookkeeper#3444 . That's probably missing from most dashboards.

nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2024
(cherry picked from commit 3ada566)
(cherry picked from commit ca4512c)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 20, 2024
(cherry picked from commit 3ada566)
(cherry picked from commit ca4512c)
grssam pushed a commit to grssam/pulsar that referenced this pull request Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] Provide a solution for configuring Bookkeeper RocksDB settings in Pulsar Kubernetes deployments
3 participants