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

fix: rocksdb options not changed even if update in Pegasus config file #1108

Merged
merged 23 commits into from
Aug 25, 2022

Conversation

WHBANG
Copy link
Contributor

@WHBANG WHBANG commented Aug 9, 2022

What problem does this PR solve?

#1025

What is changed and how does it work?

The specific reason is that if you open an existing rocksdb, the usage scenario will be set to normal first, and then set to the corresponding mode according to the actual usage scenario. This process will change the relevant options, causing rocksdb to execute flush and compact, so in order to avoid this situation (#587), rocksdb::LoadLatestOptions is called, and these parameters are initialized with the last value, so the last thing you see is that if the configuration file is modified, and without changing the usage scenario mode, these modified configurations will not take effect after restart.

Options involved:

level0_file_num_compaction_trigger
level0_slowdown_writes_trigger
level0_stop_writes_trigger
soft_pending_compaction_bytes_limit
hard_pending_compaction_bytes_limit
disable_auto_compactions
max_compaction_bytes
write_buffer_size
max_write_buffer_number

Solution:

  1. After opening rocksdb, calculate the value of the corresponding options according to the usage scenario mode in the environment variable, compare whether there is a change, and call setOpion() if there is a change to set the newly calculated value.
  2. If the usage scenario mode has been changed during this process, the options will be the correct value, and nothing needs to be done.
Tests
  • Unit test

  • Manual test (add detailed scripts or steps below)

@github-actions github-actions bot added the cpp label Aug 9, 2022
@acelyc111 acelyc111 added the type/bug-fix This PR fixes a bug. label Aug 10, 2022
src/server/pegasus_server_impl.h Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
@WHBANG WHBANG force-pushed the dev/rocksdb_options_not_change branch from 6f574c1 to b74d120 Compare August 11, 2022 13:32
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
@WHBANG WHBANG force-pushed the dev/rocksdb_options_not_change branch 4 times, most recently from 2aa9f41 to 539dab2 Compare August 16, 2022 13:36
@WHBANG WHBANG force-pushed the dev/rocksdb_options_not_change branch from 539dab2 to 497c8a9 Compare August 16, 2022 17:16
@WHBANG WHBANG force-pushed the dev/rocksdb_options_not_change branch from 497c8a9 to ad93831 Compare August 17, 2022 03:18
src/server/pegasus_server_impl.h Show resolved Hide resolved
src/server/pegasus_server_impl.h Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
empiredan
empiredan previously approved these changes Aug 19, 2022
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.h Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.h Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.h Show resolved Hide resolved
Co-authored-by: Yingchun Lai <acelyc1112009@gmail.com>
WHBANG and others added 4 commits August 19, 2022 15:23
Co-authored-by: Yingchun Lai <acelyc1112009@gmail.com>
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
acelyc111
acelyc111 previously approved these changes Aug 24, 2022
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Show resolved Hide resolved
src/server/pegasus_server_impl.cpp Outdated Show resolved Hide resolved
@acelyc111 acelyc111 merged commit 02f10e9 into apache:master Aug 25, 2022
ZhongChaoqiang pushed a commit to ZhongChaoqiang/incubator-pegasus that referenced this pull request Nov 28, 2022
@empiredan empiredan mentioned this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants